[PATCH] s3:notifyd: Handle sigup in notifyd to reparse smb.conf

[Jeremy Allison]

On Fri, May 24, 2019 at 09:04:56PM +0000, Shyam Rathi wrote:
> Hi Jeremy,
> Please find the attached update patch. I’ve addressed both review comments.

Thanks Shyam, that looks good to me. I'll push
it into gitlab-ci and if it passes create a merge
request.

Jeremy.

> Regards,
> -Shyam
> 
> From: Jeremy Allison <jra at samba.org>
> Reply-To: Jeremy Allison <jra at samba.org>
> Date: Thursday, May 23, 2019 at 7:42 PM
> To: Shyam Rathi <shyam.rathi at nutanix.com>
> Cc: "samba-technical at lists.samba.org" <samba-technical at lists.samba.org>
> Subject: Re: [PATCH] s3:notifyd: Handle sigup in notifyd to reparse smb.conf
> 
> On Fri, May 24, 2019 at 12:18:36AM +0000, Shyam Rathi via samba-technical wrote:
> Thanks for the review Jeremy. Responses inline below.
> +       DEBUG(2,("notifyd: Reloading services after SIGHUP\n"));
> 	^^^^
> 	Please use either the DBG_WARNING or DBG_NOTICE macros
> 	to ensure consistency in new debug info levels.
> 		---- I'll make the change.
> 
> Thanks !
> 
> +       become_root();
> +       reload_services(NULL, NULL, false);
> +       unbecome_root();
> 	^^^^^^^
> Does notifyd need become_root()/unbecome_root()
> pairs ? Isn't it always invoked when smbd is root
> and stays as such ?
> 		---- The sighup handler for parent smbd calls
> 			"change_to_root_user()" before calling
> 			reload_services(). For many other binaries,
> 			either reload_services() or (un)become_root
> 			is called first. I mainly kept the same
> 			behavior. I'm not fully sure about its purpose
> 			at this place.
> 
> None of the notifyd code ever changes credentials,
> so when it runs as root it'll stay that way.
> 
> smbd has to change_to_root_user() inside the signal
> handler as it often changes credentials to the current
> attached user, so might not be currently running as
> root.
> 
> I don't think you have the same problem inside notifyd
> so you can drop the become_root()/unbecome_root() I
> think.
> 
> Test it once you've made the change to make sure :-).
> 
> Jeremy.
> 
>