[PATCH] Rename SAFEALIGN macros
by Michal Židek
Hi,
about 3 months ago I renamed the SAFEALIGN macros (and added aliases
for backward compatibility) from
SAFEALIGN_COPY_<type>
SAFEALIGN_SET_<type>
to
SAFEALIGN_BUF2VAR_<type>
SAFEALIGN_VAR2BUF_<type>
according to request in ticket
https://fedorahosted.org/sssd/ticket/1772
But, now I think I used wrong new names for these macros, because the
SAFEALIGN_BUF2VAR_<type> is not meant to only copy value from buffer to
variable, it can be used the other way around as well (it is basically
just memcpy with bytes counting). So the original SAFEALIGN_COPY_<> was
IMO much better name for the macro.
The second macro SAFEALIGN_VAR2BUF_<type> is used to SET the buffer at
given address with specified value. This can be used if the
value we copy to buffer is just expression (like 3 + 5), #DEFINEd
constant or literal (so we can not get its address with &, so
SAFEALIGN_COPY_<type> can not be used). The original name of this macro
was
SAFEALIGN_SET_<type>
which was better than the new one. I wrongly interpreted the macros when
creating the new names and realized it now when I wanted to use them.
The reason why the name change was requested is most likely because
SAFEALIGN_SET_UINT32
might look like it should be used when setting uint32_t variable to
a specific value taken from buffer. The parameter 'value' is
expression of type uint32_t and not address to some uint32_t value
in buffer. Another reason why people might have been confused with the
old names was that it was not clear that with SAFEALIGN_SET the 'value'
is not a pointer to value but the value itself. I think this was the
reason why the ticket was created.
So, I would prefer to change the names once again (the new ones are not
used anywhere so far) to the original ones with one little difference.
The SAFEALIGN_SET_<type> will be SAFEALIGN_SETMEM_<type>. It makes more
clear that we want to change memory at given address (and not set some
variable of type <type>).
With additional comments showing the expected types of all arguments,
this should be clear enough and hopefully nobody will be confused in
the future.
Patch is attached.
Thanks
Michal
10 years, 7 months
[PATCH] Fix warning missing arguments
by Lukas Slebodnik
ehlo
attached patch does not fit into another big formatig patchset.
This mistake looks like copy&paste problem, but I am not sure about this
solution.
Patch is attached.
LS
10 years, 7 months
Question about pam_auth_req and crash from BZ972699
by Lukas Slebodnik
ehlo,
I have a question about struct pam_auth_req and crash from BZ972699
I thought It is a duplicate of upstream ticket
https://fedorahosted.org/sssd/ticket/2018 (use after free)
But I anlysed the core dump and it doe not look like use after free.
I also checked talloc_magic for struct "pam_auth_req" and it is ok.
Here is the definition of structure.
(gdb) ptype preq
type = struct pam_auth_req {
struct cli_ctx *cctx;
struct sss_domain_info *domain;
struct pam_data *pd;
pam_dp_callback_t *callback;
struct ldb_result *res;
_Bool check_provider;
void *data;
} *
And here is output from gdb
(gdb) p *preq
$1 = {cctx = 0x9f90a0, domain = 0x9a3740, pd = 0xa178f0, callback = 0,
res = 0x0, check_provider = false, data = 0x0}
#0 0x0000000000000000 in ?? ()
#1 0x00000000004110d1 in pam_dp_process_reply (pending=0x9d3c00,
ptr=<value optimized out>) at src/responder/pam/pamsrv_dp.c:79
#2 0x00007f1aeb0a661a in complete_pending_call_and_unlock (
connection=0xa1e3a0, pending=0x9d3c00, message=<value optimized out>)
at dbus-connection.c:2234
#3 0x00007f1aeb0a886f in dbus_connection_dispatch (connection=0xa1e3a0)
at dbus-connection.c:4397
#4 0x000000000045425e in sbus_dispatch (ev=0x99d380,
te=<value optimized out>, tv=..., data=<value optimized out>)
at src/sbus/sssd_dbus_connection.c:104
#5 0x00007f1aeb920bd9 in tevent_common_loop_timer_delay (ev=0x99d380)
at ../tevent_timed.c:254
77 dbus_pending_call_unref(pending);
78 dbus_message_unref(msg);
79 preq->callback(preq); <<<<<<< CHASH HERE
80 }
81
sssd_pam crashed because preq->callback was NULL.
I would like to describe struct pam_auth_req more deeply.
struct pam_auth_req is created in function pam_forwarder
form file "src/responder/pam/pamsrv_cmd.c".
Most of structure members are initialized in this function, but *calback*
is initialized in function pam_dom_forwarder.
1173 if (!NEED_CHECK_PROVIDER(preq->domain->provider)) {
1174 preq->callback = pam_reply; <<<<HERE
1175 ret = LOCAL_pam_handler(preq);
1176 }
1177 else {
1178 preq->callback = pam_reply; <<<<< OR HERE
1179 ret = pam_dp_send_req(preq, SSS_CLI_SOCKET_TIMEOUT/2);
1180 DEBUG(4, ("pam_dp_send_req returned %d\n", ret));
1181 }
1182
There are 3 places where is pam_dom_forwarder called and it is called only
in some condition (pam_check_user_search returned EOK)
Mostly it looks like following lines:
852 ret = pam_check_user_search(preq);
853 if (ret == EOK) {
854 pam_dom_forwarder(preq);
855 }
And my question are:
1. Do we need to initialize pam_auth_req->callback only in function
pam_dom_forwarder?
2. Should we check preq->callback for NULL?
3. Do you have any idea how to reproduce it?
I really don't know what is right solution.
Thank you very much for any ideas.
LS
10 years, 7 months
[PATCH] KRB5: Fix warning declaration shadows global declaration
by Lukas Slebodnik
ehlo,
I tested some patches on RHEL6. and there is newly introduced warning.
gcc version 4.4.7
src/providers/krb5/krb5_utils.c:193: warning: declaration of 'rewind' shadows a
global declaration
/usr/include/stdio.h:754: warning: shadowed declaration is here
Warning was introduced in patch dcc6877aa2e2dd63a9dc9c411a9c58feaeb36b9a
"krb5: Fetch ccname template from krb5.conf"
LS
10 years, 7 months
test taking long time
by Simo Sorce
The test named test_ss_idmap, takes quite long on my machine (as in
minutes) although the machine is completely idle. Any idea what causes
it ? Any way to avoid it ?
Simo.
--
Simo Sorce * Red Hat, Inc * New York
10 years, 7 months
[PATCH] UTIL: do not redefine SIZE_T_MAX
by Lukas Slebodnik
ehlo,
Some platforms can have defined SIZE_T_MAX.
It is better to use conditional build.
Two patches are attached. one for master(1.10) and second for 1.9
LS
10 years, 7 months
[PATCH] Include sys/types.h for types id_t and uid_t
by Lukas Slebodnik
ehlo,
I checked some manual pages, where [ug]?id types are used and each manual page
suggest to include header file "sys/types.h". This header file was indirectly
included in some files on linux, but it is not portable.
man getgid
#include <unistd.h>
#include <sys/types.h>
gid_t getgid(void);
gid_t getegid(void);
-------------------------
man getpwuid
#include <sys/types.h>
#include <pwd.h>
struct passwd *getpwuid(uid_t uid);
Attached patch modify header file "sss_idmap.h". Should be version-info changed
for libsssidmap?
LS
10 years, 7 months
