2020-04-02 13:52:14

by Lukas Czerner

[permalink] [raw]
Subject: [PATCH] e2scrub: Remove PATH setting from the scripts

We don't want to override system setting by changing the PATH. This
should remain under administrator/user control.

Signed-off-by: Lukas Czerner <[email protected]>
---
scrub/e2scrub.in | 2 --
scrub/e2scrub_all.in | 2 --
2 files changed, 4 deletions(-)

diff --git a/scrub/e2scrub.in b/scrub/e2scrub.in
index 30ab7cbd..7c3f46e9 100644
--- a/scrub/e2scrub.in
+++ b/scrub/e2scrub.in
@@ -23,8 +23,6 @@
# check filesystems in VGs that have at least 256MB (or so) of
# free space.

-PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
-
if (( $EUID != 0 )); then
echo "e2scrub must be run as root"
exit 1
diff --git a/scrub/e2scrub_all.in b/scrub/e2scrub_all.in
index 4288b969..b6a7d0ad 100644
--- a/scrub/e2scrub_all.in
+++ b/scrub/e2scrub_all.in
@@ -18,8 +18,6 @@
# along with this program; if not, write the Free Software Foundation,
# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.

-PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
-
if (( $EUID != 0 )); then
echo "e2scrub_all must be run as root"
exit 1
--
2.21.1


2020-04-02 15:18:02

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH] e2scrub: Remove PATH setting from the scripts

On Thu, Apr 02, 2020 at 03:47:16PM +0200, Lukas Czerner wrote:
> We don't want to override system setting by changing the PATH. This
> should remain under administrator/user control.
>
> Signed-off-by: Lukas Czerner <[email protected]>

Looks ok to me,
Reviewed-by: Darrick J. Wong <[email protected]>

--D

> ---
> scrub/e2scrub.in | 2 --
> scrub/e2scrub_all.in | 2 --
> 2 files changed, 4 deletions(-)
>
> diff --git a/scrub/e2scrub.in b/scrub/e2scrub.in
> index 30ab7cbd..7c3f46e9 100644
> --- a/scrub/e2scrub.in
> +++ b/scrub/e2scrub.in
> @@ -23,8 +23,6 @@
> # check filesystems in VGs that have at least 256MB (or so) of
> # free space.
>
> -PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
> -
> if (( $EUID != 0 )); then
> echo "e2scrub must be run as root"
> exit 1
> diff --git a/scrub/e2scrub_all.in b/scrub/e2scrub_all.in
> index 4288b969..b6a7d0ad 100644
> --- a/scrub/e2scrub_all.in
> +++ b/scrub/e2scrub_all.in
> @@ -18,8 +18,6 @@
> # along with this program; if not, write the Free Software Foundation,
> # Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.
>
> -PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
> -
> if (( $EUID != 0 )); then
> echo "e2scrub_all must be run as root"
> exit 1
> --
> 2.21.1
>

2020-04-10 16:17:02

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] e2scrub: Remove PATH setting from the scripts

On Thu, Apr 02, 2020 at 03:47:16PM +0200, Lukas Czerner wrote:
> We don't want to override system setting by changing the PATH. This
> should remain under administrator/user control.
>
> Signed-off-by: Lukas Czerner <[email protected]>

The reason why the PATH was added is because most users don't have
/sbin or /usr/sbin in their PATH, and if they run "sudo e2scrub",
finding commands like lvcreate, lvremove, et. al., wouldn't be there.

I suppose we could do something like

PATH=$PATH:/sbin:/usr/sbin

instead, but otherwise, users will see some unexpected failures.

- Ted

2020-04-16 09:39:36

by Lukas Czerner

[permalink] [raw]
Subject: Re: [PATCH] e2scrub: Remove PATH setting from the scripts

On Fri, Apr 10, 2020 at 12:16:35PM -0400, Theodore Y. Ts'o wrote:
> On Thu, Apr 02, 2020 at 03:47:16PM +0200, Lukas Czerner wrote:
> > We don't want to override system setting by changing the PATH. This
> > should remain under administrator/user control.
> >
> > Signed-off-by: Lukas Czerner <[email protected]>
>
> The reason why the PATH was added is because most users don't have
> /sbin or /usr/sbin in their PATH, and if they run "sudo e2scrub",
> finding commands like lvcreate, lvremove, et. al., wouldn't be there.

I don't understand, e2scrub should be in be in sbin as well, right ?
Besides what if such user wants to run lvcreate, or lvremove ? This
seems like a problem that should be fixed somewhere else.

>
> I suppose we could do something like
>
> PATH=$PATH:/sbin:/usr/sbin

that's better than replacing it.

-Lukas

>
> instead, but otherwise, users will see some unexpected failures.
>
> - Ted
>