samba-technical@lists.samba.org
[Top] [All Lists]

Re: should set_unix_security_ctx() be returning an error?

Subject: Re: should set_unix_security_ctx() be returning an error?
From: Tim Prouty
Date: Thu, 13 Sep 2007 10:00:04 -0700
Hi,

After talking with James and BaT on IRC yesterday, I wrote a small patch to panic on sys_setgroups() failure. This applies cleanly to SAMBA_3_0 and SAMBA_3_2.

Index: smbd/sec_ctx.c
===================================================================
--- smbd/sec_ctx.c      (revision 25072)
+++ smbd/sec_ctx.c      (working copy)
@@ -239,7 +239,8 @@ static void set_unix_security_ctx(uid_t
        /* Start context switch */
        gain_root();
#ifdef HAVE_SETGROUPS
-       sys_setgroups(gid, ngroups, groups);
+       if (sys_setgroups(gid, ngroups, groups) != 0)
+               smb_panic("sys_setgroups failed");
#endif
        become_id(uid, gid);
        /* end context switch */


-Tim

On Sep 11, 2007, at 2:46 PM, Tim Prouty wrote:

Jeremy and James,

We recently ran into the setgroups bug that you fixed in June. I was reviewing the patches in the process of applying them, and I have a question about set_unix_security_ctx(). Currently it is void, and the comment claims that it will panic if not successful.

sys_setgroups() returns an int that could be non-zero if:
1) the setgroups() syscall fails.
2) malloc() fails.

Should set_unix_security_ctx() actually be returning an int or BOOL, so set_sec_ctx() and pop_sec_ctx() have the opportunity to handle the error? An error returned from setgroups() indicates an error in the logic of the code, and we should probably panic. What about the failed malloc? Should there smb_panic() be called in this case as well?

Thanks!

-Tim

Tim Prouty | Software Development Engineer
Isilon Systems    P +1-206-315-7500     F  +1-206-315-7485
www.isilon.com    D +1-206-315-7494    M +1-206-963-5747





<Prev in Thread] Current Thread [Next in Thread>