* Re: Subject: [PATCH] Statd should always 'chdir' to its state directory.
[not found] <20110721104032.38e289aa@notabene.brown>
@ 2011-07-21 9:05 ` Chuck Lever
2011-07-21 19:26 ` Steve Dickson
1 sibling, 0 replies; 2+ messages in thread
From: Chuck Lever @ 2011-07-21 9:05 UTC (permalink / raw)
To: NeilBrown; +Cc: Steve Dickson, linux-nfs@vger.kernel.org
On Jul 21, 2011, at 1:40 AM, NeilBrown wrote:
>
> Subject: [PATCH] Statd should always 'chdir' to its state directory.
>
> As statd can be started by 'mount' which can sometimes be run by a
> normal user, the current-working-directory could be anything. In
> partcular it could be in a mounted filesystem. As 'statd' continues
> running as a daemon it could keep prevent that filesystem from being
> unmounted.
>
> statd does currently 'chdir' to the state directory, but only if the
> state directory is not owned by root. This is wrong - it should check
> for root after the chdir, not before.
>
> So swap the two if statements around.
Seems reasonable.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
>
> --
> looks like this regression slipped in around January 2010 with the big libnsm clean up.
> -NB
>
>
> diff --git a/support/nsm/file.c b/support/nsm/file.c
> index 98b47bf..a12c753 100644
> --- a/support/nsm/file.c
> +++ b/support/nsm/file.c
> @@ -395,18 +395,18 @@ nsm_drop_privileges(const int pidfd)
> return false;
> }
>
> - if (st.st_uid == 0) {
> - xlog_warn("Running as root. "
> - "chown %s to choose different user", nsm_base_dirname);
> - return true;
> - }
> -
> if (chdir(nsm_base_dirname) == -1) {
> xlog(L_ERROR, "Failed to change working directory to %s: %m",
> nsm_base_dirname);
> return false;
> }
>
> + if (st.st_uid == 0) {
> + xlog_warn("Running as root. "
> + "chown %s to choose different user", nsm_base_dirname);
> + return true;
> + }
> +
> /*
> * If the pidfile happens to reside on NFS, dropping privileges
> * will probably cause us to lose access, even though we are
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: Subject: [PATCH] Statd should always 'chdir' to its state directory.
[not found] <20110721104032.38e289aa@notabene.brown>
2011-07-21 9:05 ` Subject: [PATCH] Statd should always 'chdir' to its state directory Chuck Lever
@ 2011-07-21 19:26 ` Steve Dickson
1 sibling, 0 replies; 2+ messages in thread
From: Steve Dickson @ 2011-07-21 19:26 UTC (permalink / raw)
To: NeilBrown; +Cc: Chuck Lever, linux-nfs@vger.kernel.org
On 07/20/2011 08:40 PM, NeilBrown wrote:
>
> Subject: [PATCH] Statd should always 'chdir' to its state directory.
>
> As statd can be started by 'mount' which can sometimes be run by a
> normal user, the current-working-directory could be anything. In
> partcular it could be in a mounted filesystem. As 'statd' continues
> running as a daemon it could keep prevent that filesystem from being
> unmounted.
>
> statd does currently 'chdir' to the state directory, but only if the
> state directory is not owned by root. This is wrong - it should check
> for root after the chdir, not before.
>
> So swap the two if statements around.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
Committed...
steved.
>
> --
> looks like this regression slipped in around January 2010 with the big libnsm clean up.
> -NB
>
>
> diff --git a/support/nsm/file.c b/support/nsm/file.c
> index 98b47bf..a12c753 100644
> --- a/support/nsm/file.c
> +++ b/support/nsm/file.c
> @@ -395,18 +395,18 @@ nsm_drop_privileges(const int pidfd)
> return false;
> }
>
> - if (st.st_uid == 0) {
> - xlog_warn("Running as root. "
> - "chown %s to choose different user", nsm_base_dirname);
> - return true;
> - }
> -
> if (chdir(nsm_base_dirname) == -1) {
> xlog(L_ERROR, "Failed to change working directory to %s: %m",
> nsm_base_dirname);
> return false;
> }
>
> + if (st.st_uid == 0) {
> + xlog_warn("Running as root. "
> + "chown %s to choose different user", nsm_base_dirname);
> + return true;
> + }
> +
> /*
> * If the pidfile happens to reside on NFS, dropping privileges
> * will probably cause us to lose access, even though we are
^ permalink raw reply [flat|nested] 2+ messages in thread