should set_unix_security_ctx() be returning an error?

Tim Prouty tim.prouty at isilon.com
Thu Sep 13 22:23:29 GMT 2007


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
>






More information about the samba-technical mailing list