PATCH: update smbrun and related to set environment variables when need

David Disseldorp ddiss at suse.de
Wed Oct 12 11:28:24 UTC 2016


Hi Trevor,

Thanks for your efforts in trying to get vfs_virusfilter merged!
I just have a few minor nits for this change...

On Wed, 12 Oct 2016 01:40:42 -0600, Trever L. Adams wrote:

> diff --git a/source3/passdb/pdb_interface.c b/source3/passdb/pdb_interface.c
> index 5260320..04927fb 100644
> --- a/source3/passdb/pdb_interface.c
> +++ b/source3/passdb/pdb_interface.c
> @@ -485,7 +485,7 @@ static NTSTATUS pdb_default_create_user(struct pdb_methods *methods,
>  		if (!add_script) {
>  			return NT_STATUS_NO_MEMORY;
>  		}
> -		add_ret = smbrun(add_script,NULL);
> +		add_ret = smbrun(add_script,NULL,NULL);

Please fix the spacing here while you're at it, and in the other callers
without a space after comma.

> -	if ( (ret = smbrun(command, NULL)) == 0 ) {
> +	if ( (ret = smbrun(command, NULL, NULL)) == 0 ) {
>  		/* Tell everyone we updated smb.conf. */
>  		message_send_all(msg_ctx, MSG_SMB_CONF_UPDATED, NULL, 0, NULL);
>  	}

Might as well split this into:
	ret = smbrun(command, NULL, NULL);
	if (ret == 0) {
		...
Please do this for the other callers as well as well.

> @@ -626,7 +626,7 @@ WERROR _winreg_AbortSystemShutdown(struct pipes_struct *p,
>  	if ( can_shutdown )
>  		become_root();
>  
> -	ret = smbrun( abort_shutdown_script, NULL );
> +	ret = smbrun( abort_shutdown_script, NULL, NULL );

Drop the extra '( ' and ' )' spaces here, and in the other cases.

> -static int smbrun_internal(const char *cmd, int *outfd, bool sanitize)
> +static int smbrun_internal(const char *cmd, int *outfd, bool sanitize,
> +	char * const *env)
>  {
>  	pid_t pid;
>  	uid_t uid = current_user.ut.uid;
> @@ -197,7 +198,11 @@ static int smbrun_internal(const char *cmd, int *outfd, bool sanitize)
>  				exit(82);
>  		}
>  
> -		execl("/bin/sh","sh","-c",
> +		if (env)
> +			execle("/bin/sh","sh","-c",
> +			    newcmd ? (const char *)newcmd : cmd, NULL,
> +			    env);
> +		else execl("/bin/sh","sh","-c",
>  		    newcmd ? (const char *)newcmd : cmd, NULL);

Please use:
	if (env != NULL) {
		...
	} else {
		...
	}

> -int smbrun_no_sanitize(const char *cmd, int *outfd)
> +int smbrun_no_sanitize(const char *cmd, int *outfd, char * const *env)
>  {
> -	return smbrun_internal(cmd, outfd, False);
> +	return smbrun_internal(cmd, outfd, False, env);

s/False/false/ while you're here.

>  }
>  
>  /****************************************************************************
>   By default this now sanitizes shell expansion.
>  ****************************************************************************/
>  
> -int smbrun(const char *cmd, int *outfd)
> +int smbrun(const char *cmd, int *outfd, char * const *env)
>  {
> -	return smbrun_internal(cmd, outfd, True);
> +	return smbrun_internal(cmd, outfd, True, env);

s/True/true/ while you're here too.

Otherwise I think this patch looks fine.

Cheers, David



More information about the samba-technical mailing list