[LTP] [PATCH] runtest/fs: Don't read files in /dev indiscriminately

[LTP] [PATCH] runtest/fs: Don't read files in /dev indiscriminately

[Punit Agrawal]

read_all_dev attempts to read 1024 bytes from all devices in /dev. As
nodes in /dev represent devices, any access can have side-effects -
sometimes fatally so, e.g., accessing /dev/port on Juno R2 and AMD
Seattle lead to synchronous external abort or SError (system error)
interrupt depending on the access pattern.

There isn't much the kernel can do about the aborts other than panic
the system.

The side-effects problem is also highlighted by the recent exclusion
added for watchdog devices. See commit 4a41aa6b48c134e ("runtest/fs:
filter /dev/watchdog* for read_all_dev by default").

It would be better to replace indiscriminate reading of /dev files
with tests targeting specific files in /dev which have defined known
behaviour, e.g., /dev/null, /dev/urandom, etc.

In the meanwhile, drop the indiscriminate reading of files in /dev.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: xuyang.jy@cn.fujitsu.com
Cc: naresh.kamboju@linaro.org
Cc: rpalethorpe@suse.com
Cc: chrubis@suse.cz
Cc: james.morse@arm.com
---
Hi,

The test leads to panic during nightly ltp runs on internal
systems. Looking at the crash report from Naresh[0], it looks likely
that he's facing the same problem.

Please consider including in upcoming release.

Thanks,
Punit

[0] http://lists.linux.it/pipermail/ltp/2018-May/007954.html
---
 runtest/fs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/runtest/fs b/runtest/fs
index 42a9bfcbf..c7ed64fbf 100644
--- a/runtest/fs
+++ b/runtest/fs
@@ -69,7 +69,6 @@ fs_di fs_di -d $TMPDIR
 # Was not sure why it should reside in runtest/crashme and won´t get tested ever
 proc01 proc01 -m 128
 
-read_all_dev read_all -d /dev -e '/dev/watchdog?(0)' -q -r 10
 read_all_proc read_all -d /proc -q -r 10
 read_all_sys read_all -d /sys -q -r 10
 
-- 
2.17.0

[LTP] [PATCH] runtest/fs: Don't read files in /dev indiscriminately

[Richard Palethorpe]

Hello,

Punit Agrawal writes:

> read_all_dev attempts to read 1024 bytes from all devices in /dev. As
> nodes in /dev represent devices, any access can have side-effects -
> sometimes fatally so, e.g., accessing /dev/port on Juno R2 and AMD
> Seattle lead to synchronous external abort or SError (system error)
> interrupt depending on the access pattern.
>
> There isn't much the kernel can do about the aborts other than panic
> the system.
>
> The side-effects problem is also highlighted by the recent exclusion
> added for watchdog devices. See commit 4a41aa6b48c134e ("runtest/fs:
> filter /dev/watchdog* for read_all_dev by default").
>
> It would be better to replace indiscriminate reading of /dev files
> with tests targeting specific files in /dev which have defined known
> behaviour, e.g., /dev/null, /dev/urandom, etc.
>
> In the meanwhile, drop the indiscriminate reading of files in /dev.

Another solution might be to add an option which drops privileges before
running the test on /dev. An unprivileged user should not have access to files
like /dev/port, but will for /dev/null and /dev/urandom.

>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> Cc: xuyang.jy@cn.fujitsu.com
> Cc: naresh.kamboju@linaro.org
> Cc: rpalethorpe@suse.com
> Cc: chrubis@suse.cz
> Cc: james.morse@arm.com
> ---
> Hi,
>
> The test leads to panic during nightly ltp runs on internal
> systems. Looking at the crash report from Naresh[0], it looks likely
> that he's facing the same problem.
>
> Please consider including in upcoming release.
>
> Thanks,
> Punit
>
> [0] http://lists.linux.it/pipermail/ltp/2018-May/007954.html
> ---
>  runtest/fs | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/runtest/fs b/runtest/fs
> index 42a9bfcbf..c7ed64fbf 100644
> --- a/runtest/fs
> +++ b/runtest/fs
> @@ -69,7 +69,6 @@ fs_di fs_di -d $TMPDIR
>  # Was not sure why it should reside in runtest/crashme and won´t get tested ever
>  proc01 proc01 -m 128
>
> -read_all_dev read_all -d /dev -e '/dev/watchdog?(0)' -q -r 10
>  read_all_proc read_all -d /proc -q -r 10
>  read_all_sys read_all -d /sys -q -r 10


--
Thank you,
Richard.

[LTP] [PATCH] runtest/fs: Don't read files in /dev indiscriminately

[Cyril Hrubis]

Hi!
> > It would be better to replace indiscriminate reading of /dev files
> > with tests targeting specific files in /dev which have defined known
> > behaviour, e.g., /dev/null, /dev/urandom, etc.
> >
> > In the meanwhile, drop the indiscriminate reading of files in /dev.
> 
> Another solution might be to add an option which drops privileges before
> running the test on /dev. An unprivileged user should not have access to files
> like /dev/port, but will for /dev/null and /dev/urandom.

That sounds like a better solution to me, can we get that in before the
release?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] runtest/fs: Don't read files in /dev indiscriminately

[Richard Palethorpe]

Hello,

Cyril Hrubis writes:

> Hi!
>> > It would be better to replace indiscriminate reading of /dev files
>> > with tests targeting specific files in /dev which have defined known
>> > behaviour, e.g., /dev/null, /dev/urandom, etc.
>> >
>> > In the meanwhile, drop the indiscriminate reading of files in /dev.
>> 
>> Another solution might be to add an option which drops privileges before
>> running the test on /dev. An unprivileged user should not have access to files
>> like /dev/port, but will for /dev/null and /dev/urandom.
>
> That sounds like a better solution to me, can we get that in before the
> release?

Yes, I can give this priority.

-- 
Thank you,
Richard.

[LTP] [PATCH] runtest/fs: Don't read files in /dev indiscriminately

[Punit Agrawal]

Hi Richard,

Richard Palethorpe <rpalethorpe@suse.de> writes:

> Hello,
>
> Punit Agrawal writes:
>
>> read_all_dev attempts to read 1024 bytes from all devices in /dev. As
>> nodes in /dev represent devices, any access can have side-effects -
>> sometimes fatally so, e.g., accessing /dev/port on Juno R2 and AMD
>> Seattle lead to synchronous external abort or SError (system error)
>> interrupt depending on the access pattern.
>>
>> There isn't much the kernel can do about the aborts other than panic
>> the system.
>>
>> The side-effects problem is also highlighted by the recent exclusion
>> added for watchdog devices. See commit 4a41aa6b48c134e ("runtest/fs:
>> filter /dev/watchdog* for read_all_dev by default").
>>
>> It would be better to replace indiscriminate reading of /dev files
>> with tests targeting specific files in /dev which have defined known
>> behaviour, e.g., /dev/null, /dev/urandom, etc.
>>
>> In the meanwhile, drop the indiscriminate reading of files in /dev.
>
> Another solution might be to add an option which drops privileges before
> running the test on /dev. An unprivileged user should not have access to files
> like /dev/port, but will for /dev/null and /dev/urandom.

The following files have unprivileged read permission in /dev -

vfio/vfio
net/tun
ptmx
tty
urandom
random
full
zero
null

which seems a reasonable set (this'll likely vary by system).

So dropping privileges should keep things ticking.

Thanks,
Punit


>
>>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> Cc: xuyang.jy@cn.fujitsu.com
>> Cc: naresh.kamboju@linaro.org
>> Cc: rpalethorpe@suse.com
>> Cc: chrubis@suse.cz
>> Cc: james.morse@arm.com
>> ---
>> Hi,
>>
>> The test leads to panic during nightly ltp runs on internal
>> systems. Looking at the crash report from Naresh[0], it looks likely
>> that he's facing the same problem.
>>
>> Please consider including in upcoming release.
>>
>> Thanks,
>> Punit
>>
>> [0] http://lists.linux.it/pipermail/ltp/2018-May/007954.html
>> ---
>>  runtest/fs | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/runtest/fs b/runtest/fs
>> index 42a9bfcbf..c7ed64fbf 100644
>> --- a/runtest/fs
>> +++ b/runtest/fs
>> @@ -69,7 +69,6 @@ fs_di fs_di -d $TMPDIR
>>  # Was not sure why it should reside in runtest/crashme and won´t get tested ever
>>  proc01 proc01 -m 128
>>
>> -read_all_dev read_all -d /dev -e '/dev/watchdog?(0)' -q -r 10
>>  read_all_proc read_all -d /proc -q -r 10
>>  read_all_sys read_all -d /sys -q -r 10
>
>
> --
> Thank you,
> Richard.