[PATCH] Simplify chroot code in smbd

[Jeremy Allison]

On Wed, Feb 10, 2016 at 04:25:29PM +0100, Andreas Schneider wrote:
> Review and push appreciated ...
> 
> Thanks,
> 
> 
> 	-- andreas

Andreas - I think this:

	chroot_dir = lp_root_directory(talloc_tos());
	if (chroot_dir != NULL) {

is incorrect. lp_XXX() strings don't return NULL
if a parameter is unset, they return a pointer to '\0'.

So this should be:

	chroot_dir = lp_root_directory(talloc_tos());
	if (*chroot_dir != '\0') {

> From 2afc034fb7857b96702186327b6f8d75562f478a Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Wed, 10 Feb 2016 16:19:56 +0100
> Subject: [PATCH] smbd: Simplify chroot option in smbd
> 
> rpmlint has a check for this and prefers to call chdir() before
> chroot(). If not it will complain with
> missing-call-to-chdir-with-chroot. The old code equivalent secure. See
> 
>     http://unixwiz.net/techtips/chroot-practices.html
> 
> This removes several unneeded talloc_tos() calls.
> 
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
>  source3/smbd/process.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/source3/smbd/process.c b/source3/smbd/process.c
> index 25c6d05..8668ee4 100644
> --- a/source3/smbd/process.c
> +++ b/source3/smbd/process.c
> @@ -3903,6 +3903,8 @@ void smbd_process(struct tevent_context *ev_ctx,
>  	NTSTATUS status;
>  	struct timeval tv = timeval_current();
>  	NTTIME now = timeval_to_nttime(&tv);
> +	const char *chroot_dir = NULL;
> +	int rc;
>  
>  	status = smbXsrv_client_create(ev_ctx, ev_ctx, msg_ctx, now, &client);
>  	if (!NT_STATUS_IS_OK(status)) {
> @@ -4024,17 +4026,22 @@ void smbd_process(struct tevent_context *ev_ctx,
>  		exit_server("Could not open account policy tdb.\n");
>  	}
>  
> -	if (*lp_root_directory(talloc_tos())) {
> -		if (chroot(lp_root_directory(talloc_tos())) != 0) {
> -			DEBUG(0,("Failed to change root to %s\n",
> -				 lp_root_directory(talloc_tos())));
> -			exit_server("Failed to chroot()");
> +	chroot_dir = lp_root_directory(talloc_tos());
> +	if (chroot_dir != NULL) {
> +		rc = chdir(chroot_dir);
> +		if (rc != 0) {
> +			DBG_ERR("Failed to chdir to %s\n", chroot_dir);
> +			exit_server("Failed to chdir()");
>  		}
> -		if (chdir("/") == -1) {
> -			DEBUG(0,("Failed to chdir to / on chroot to %s\n", lp_root_directory(talloc_tos())));
> +
> +		rc = chroot(chroot_dir);
> +		if (rc != 0) {
> +			DBG_ERR("Failed to change root to %s\n", chroot_dir);
>  			exit_server("Failed to chroot()");
>  		}
> -		DEBUG(0,("Changed root to %s\n", lp_root_directory(talloc_tos())));
> +		DBG_WARNING("Changed root to %s\n", chroot_dir);
> +
> +		TALLOC_FREE(chroot_dir);
>  	}
>  
>  	if (!file_init(sconn)) {
> -- 
> 2.7.1
>