* [LTP] [PATCH v2 1/2] read_all: move blacklist to source
@ 2019-11-05 12:23 Jan Stancek
2019-11-05 12:23 ` [LTP] [PATCH v2 2/2] read_all_sys: skip debugfs Jan Stancek
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Jan Stancek @ 2019-11-05 12:23 UTC (permalink / raw)
To: ltp
library doesn't support multiple parameters for same option.
Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
runtest/fs | 2 +-
testcases/kernel/fs/read_all/read_all.c | 28 ++++++++++++++++++++++------
2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/runtest/fs b/runtest/fs
index 07d6e2b67964..46318575653e 100644
--- a/runtest/fs
+++ b/runtest/fs
@@ -71,7 +71,7 @@ proc01 proc01 -m 128
read_all_dev read_all -d /dev -p -q -r 10
read_all_proc read_all -d /proc -q -r 10
-read_all_sys read_all -d /sys -q -r 10 -e /sys/power/wakeup_count
+read_all_sys read_all -d /sys -q -r 10
#Run the File System Race Condition Check tests as well
fs_racer fs_racer.sh -t 5
diff --git a/testcases/kernel/fs/read_all/read_all.c b/testcases/kernel/fs/read_all/read_all.c
index 4edccff03a4f..46f88af2270c 100644
--- a/testcases/kernel/fs/read_all/read_all.c
+++ b/testcases/kernel/fs/read_all/read_all.c
@@ -71,7 +71,6 @@ enum dent_action {
static char *verbose;
static char *quiet;
static char *root_dir;
-static char *exclude;
static char *str_reads;
static int reads = 1;
static char *str_worker_count;
@@ -81,6 +80,11 @@ static long max_workers = 15;
static struct worker *workers;
static char *drop_privs;
+static char *blacklist[] = {
+ NULL, /* reserved for -e parameter */
+ "/sys/power/wakeup_count",
+};
+
static struct tst_option options[] = {
{"v", &verbose,
"-v Print information about successful reads."},
@@ -88,7 +92,7 @@ static struct tst_option options[] = {
"-q Don't print file read or open errors."},
{"d:", &root_dir,
"-d path Path to the directory to read from, defaults to /sys."},
- {"e:", &exclude,
+ {"e:", &blacklist[0],
"-e pattern Ignore files which match an 'extended' pattern, see fnmatch(3)."},
{"r:", &str_reads,
"-r count The number of times to schedule a file for reading."},
@@ -182,17 +186,29 @@ static void sanitize_str(char *buf, ssize_t count)
strcpy(buf + MAX_DISPLAY, "...");
}
+static int is_blacklisted(const char *path)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
+ if (blacklist[i] && !fnmatch(blacklist[i], path, FNM_EXTMATCH)) {
+ if (verbose)
+ tst_res(TINFO, "Ignoring %s", path);
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
static void read_test(const char *path)
{
char buf[BUFFER_SIZE];
int fd;
ssize_t count;
- if (exclude && !fnmatch(exclude, path, FNM_EXTMATCH)) {
- if (verbose)
- tst_res(TINFO, "Ignoring %s", path);
+ if (is_blacklisted(path))
return;
- }
if (verbose)
tst_res(TINFO, "%s(%s)", __func__, path);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 2/2] read_all_sys: skip debugfs
2019-11-05 12:23 [LTP] [PATCH v2 1/2] read_all: move blacklist to source Jan Stancek
@ 2019-11-05 12:23 ` Jan Stancek
2019-11-05 12:38 ` [LTP] [PATCH v2 1/2] read_all: move blacklist to source Cyril Hrubis
2019-11-06 12:33 ` Richard Palethorpe
2 siblings, 0 replies; 11+ messages in thread
From: Jan Stancek @ 2019-11-05 12:23 UTC (permalink / raw)
To: ltp
debugfs is meant for debugging, it exposes also device registers
and can pretty much do anything:
https://lore.kernel.org/linux-arm-kernel/1507592549.3785589.1570404050459.JavaMail.zimbra@redhat.com/
https://lore.kernel.org/stable/2029139028.10333037.1572874551626.JavaMail.zimbra@redhat.com/
Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
testcases/kernel/fs/read_all/read_all.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/testcases/kernel/fs/read_all/read_all.c b/testcases/kernel/fs/read_all/read_all.c
index 46f88af2270c..68fc7d031f80 100644
--- a/testcases/kernel/fs/read_all/read_all.c
+++ b/testcases/kernel/fs/read_all/read_all.c
@@ -83,6 +83,7 @@ static char *drop_privs;
static char *blacklist[] = {
NULL, /* reserved for -e parameter */
"/sys/power/wakeup_count",
+ "/sys/kernel/debug/*",
};
static struct tst_option options[] = {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/2] read_all: move blacklist to source
2019-11-05 12:23 [LTP] [PATCH v2 1/2] read_all: move blacklist to source Jan Stancek
2019-11-05 12:23 ` [LTP] [PATCH v2 2/2] read_all_sys: skip debugfs Jan Stancek
@ 2019-11-05 12:38 ` Cyril Hrubis
2019-11-05 13:12 ` Jan Stancek
2019-11-06 12:33 ` Richard Palethorpe
2 siblings, 1 reply; 11+ messages in thread
From: Cyril Hrubis @ 2019-11-05 12:38 UTC (permalink / raw)
To: ltp
Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/2] read_all: move blacklist to source
2019-11-05 12:38 ` [LTP] [PATCH v2 1/2] read_all: move blacklist to source Cyril Hrubis
@ 2019-11-05 13:12 ` Jan Stancek
2019-11-05 13:14 ` Cyril Hrubis
0 siblings, 1 reply; 11+ messages in thread
From: Jan Stancek @ 2019-11-05 13:12 UTC (permalink / raw)
To: ltp
----- Original Message -----
> Hi!
>
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
>
OK to push both, or is this only for first patch?
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/2] read_all: move blacklist to source
2019-11-05 13:12 ` Jan Stancek
@ 2019-11-05 13:14 ` Cyril Hrubis
2019-11-05 13:29 ` Jan Stancek
0 siblings, 1 reply; 11+ messages in thread
From: Cyril Hrubis @ 2019-11-05 13:14 UTC (permalink / raw)
To: ltp
Hi!
> OK to push both, or is this only for first patch?
Both of them, sorry for not being clear.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/2] read_all: move blacklist to source
2019-11-05 13:14 ` Cyril Hrubis
@ 2019-11-05 13:29 ` Jan Stancek
0 siblings, 0 replies; 11+ messages in thread
From: Jan Stancek @ 2019-11-05 13:29 UTC (permalink / raw)
To: ltp
----- Original Message -----
> Hi!
> > OK to push both, or is this only for first patch?
>
> Both of them, sorry for not being clear.
Thanks, both pushed.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/2] read_all: move blacklist to source
2019-11-05 12:23 [LTP] [PATCH v2 1/2] read_all: move blacklist to source Jan Stancek
2019-11-05 12:23 ` [LTP] [PATCH v2 2/2] read_all_sys: skip debugfs Jan Stancek
2019-11-05 12:38 ` [LTP] [PATCH v2 1/2] read_all: move blacklist to source Cyril Hrubis
@ 2019-11-06 12:33 ` Richard Palethorpe
2019-11-06 13:27 ` Cyril Hrubis
2019-11-06 15:28 ` Jan Stancek
2 siblings, 2 replies; 11+ messages in thread
From: Richard Palethorpe @ 2019-11-06 12:33 UTC (permalink / raw)
To: ltp
Hello,
Jan Stancek <jstancek@redhat.com> writes:
> library doesn't support multiple parameters for same option.
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
> runtest/fs | 2 +-
> testcases/kernel/fs/read_all/read_all.c | 28 ++++++++++++++++++++++------
> 2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/runtest/fs b/runtest/fs
> index 07d6e2b67964..46318575653e 100644
> --- a/runtest/fs
> +++ b/runtest/fs
> @@ -71,7 +71,7 @@ proc01 proc01 -m 128
>
> read_all_dev read_all -d /dev -p -q -r 10
> read_all_proc read_all -d /proc -q -r 10
> -read_all_sys read_all -d /sys -q -r 10 -e /sys/power/wakeup_count
> +read_all_sys read_all -d /sys -q -r 10
>
> #Run the File System Race Condition Check tests as well
> fs_racer fs_racer.sh -t 5
> diff --git a/testcases/kernel/fs/read_all/read_all.c b/testcases/kernel/fs/read_all/read_all.c
> index 4edccff03a4f..46f88af2270c 100644
> --- a/testcases/kernel/fs/read_all/read_all.c
> +++ b/testcases/kernel/fs/read_all/read_all.c
> @@ -71,7 +71,6 @@ enum dent_action {
> static char *verbose;
> static char *quiet;
> static char *root_dir;
> -static char *exclude;
> static char *str_reads;
> static int reads = 1;
> static char *str_worker_count;
> @@ -81,6 +80,11 @@ static long max_workers = 15;
> static struct worker *workers;
> static char *drop_privs;
>
> +static char *blacklist[] = {
> + NULL, /* reserved for -e parameter */
> + "/sys/power/wakeup_count",
> +};
The problem with this is that it is only required if we are running as a
privileged user. If -p is specified then it would be a bug if nobody can
read from any of these files.
So I guess we could disable the builtin blacklist if drop_privs (switch
to nobody) is specified and run this test twice on /sys with and without
-p.
> +
> static struct tst_option options[] = {
> {"v", &verbose,
> "-v Print information about successful reads."},
> @@ -88,7 +92,7 @@ static struct tst_option options[] = {
> "-q Don't print file read or open errors."},
> {"d:", &root_dir,
> "-d path Path to the directory to read from, defaults to /sys."},
> - {"e:", &exclude,
> + {"e:", &blacklist[0],
> "-e pattern Ignore files which match an 'extended' pattern, see fnmatch(3)."},
> {"r:", &str_reads,
> "-r count The number of times to schedule a file for reading."},
> @@ -182,17 +186,29 @@ static void sanitize_str(char *buf, ssize_t count)
> strcpy(buf + MAX_DISPLAY, "...");
> }
>
> +static int is_blacklisted(const char *path)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
> + if (blacklist[i] && !fnmatch(blacklist[i], path, FNM_EXTMATCH)) {
> + if (verbose)
> + tst_res(TINFO, "Ignoring %s", path);
> + return 1;
> + }
> + }
> +
> + return 0;
> +}
> +
> static void read_test(const char *path)
> {
> char buf[BUFFER_SIZE];
> int fd;
> ssize_t count;
>
> - if (exclude && !fnmatch(exclude, path, FNM_EXTMATCH)) {
> - if (verbose)
> - tst_res(TINFO, "Ignoring %s", path);
> + if (is_blacklisted(path))
> return;
> - }
>
> if (verbose)
> tst_res(TINFO, "%s(%s)", __func__, path);
> --
> 1.8.3.1
--
Thank you,
Richard.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/2] read_all: move blacklist to source
2019-11-06 12:33 ` Richard Palethorpe
@ 2019-11-06 13:27 ` Cyril Hrubis
2019-11-06 14:40 ` Richard Palethorpe
2019-11-06 15:28 ` Jan Stancek
1 sibling, 1 reply; 11+ messages in thread
From: Cyril Hrubis @ 2019-11-06 13:27 UTC (permalink / raw)
To: ltp
Hi!
> > +static char *blacklist[] = {
> > + NULL, /* reserved for -e parameter */
> > + "/sys/power/wakeup_count",
> > +};
>
> The problem with this is that it is only required if we are running as a
> privileged user. If -p is specified then it would be a bug if nobody can
> read from any of these files.
Good point.
> So I guess we could disable the builtin blacklist if drop_privs (switch
> to nobody) is specified and run this test twice on /sys with and without
Sounds reasonable, will you send a patch?
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/2] read_all: move blacklist to source
2019-11-06 13:27 ` Cyril Hrubis
@ 2019-11-06 14:40 ` Richard Palethorpe
0 siblings, 0 replies; 11+ messages in thread
From: Richard Palethorpe @ 2019-11-06 14:40 UTC (permalink / raw)
To: ltp
hello,
Cyril Hrubis <chrubis@suse.cz> writes:
> Hi!
>> > +static char *blacklist[] = {
>> > + NULL, /* reserved for -e parameter */
>> > + "/sys/power/wakeup_count",
>> > +};
>>
>> The problem with this is that it is only required if we are running as a
>> privileged user. If -p is specified then it would be a bug if nobody can
>> read from any of these files.
>
> Good point.
>
>> So I guess we could disable the builtin blacklist if drop_privs (switch
>> to nobody) is specified and run this test twice on /sys with and without
>
> Sounds reasonable, will you send a patch?
Yeah, sure.
--
Thank you,
Richard.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/2] read_all: move blacklist to source
2019-11-06 12:33 ` Richard Palethorpe
2019-11-06 13:27 ` Cyril Hrubis
@ 2019-11-06 15:28 ` Jan Stancek
2019-11-07 10:33 ` Richard Palethorpe
1 sibling, 1 reply; 11+ messages in thread
From: Jan Stancek @ 2019-11-06 15:28 UTC (permalink / raw)
To: ltp
----- Original Message -----
> > +static char *blacklist[] = {
> > + NULL, /* reserved for -e parameter */
> > + "/sys/power/wakeup_count",
> > +};
>
> The problem with this is that it is only required if we are running as a
> privileged user. If -p is specified then it would be a bug if nobody can
> read from any of these files.
>
> So I guess we could disable the builtin blacklist if drop_privs (switch
> to nobody) is specified
Good point. I just saw your reply that you plan to send a patch, thank you.
> and run this test twice on /sys with and without
> -p.
greg-kh wasn't very happy to hear about privileged runs in the other thread.
He was suggesting whitelist approach, but I don't know how we would keep it
up to date, deal with different configs, etc.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/2] read_all: move blacklist to source
2019-11-06 15:28 ` Jan Stancek
@ 2019-11-07 10:33 ` Richard Palethorpe
0 siblings, 0 replies; 11+ messages in thread
From: Richard Palethorpe @ 2019-11-07 10:33 UTC (permalink / raw)
To: ltp
Hello,
Jan Stancek <jstancek@redhat.com> writes:
> ----- Original Message -----
>> > +static char *blacklist[] = {
>> > + NULL, /* reserved for -e parameter */
>> > + "/sys/power/wakeup_count",
>> > +};
>>
>> The problem with this is that it is only required if we are running as a
>> privileged user. If -p is specified then it would be a bug if nobody can
>> read from any of these files.
>>
>> So I guess we could disable the builtin blacklist if drop_privs (switch
>> to nobody) is specified
>
> Good point. I just saw your reply that you plan to send a patch, thank
> you.
Thanks!
>
>> and run this test twice on /sys with and without
>> -p.
>
> greg-kh wasn't very happy to hear about privileged runs in the other thread.
> He was suggesting whitelist approach, but I don't know how we would keep it
> up to date, deal with different configs, etc.
--
Thank you,
Richard.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-11-07 10:33 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05 12:23 [LTP] [PATCH v2 1/2] read_all: move blacklist to source Jan Stancek
2019-11-05 12:23 ` [LTP] [PATCH v2 2/2] read_all_sys: skip debugfs Jan Stancek
2019-11-05 12:38 ` [LTP] [PATCH v2 1/2] read_all: move blacklist to source Cyril Hrubis
2019-11-05 13:12 ` Jan Stancek
2019-11-05 13:14 ` Cyril Hrubis
2019-11-05 13:29 ` Jan Stancek
2019-11-06 12:33 ` Richard Palethorpe
2019-11-06 13:27 ` Cyril Hrubis
2019-11-06 14:40 ` Richard Palethorpe
2019-11-06 15:28 ` Jan Stancek
2019-11-07 10:33 ` Richard Palethorpe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.