* [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers
@ 2019-02-20 23:32 Eric Sandeen
2019-02-20 23:35 ` Eric Sandeen
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Eric Sandeen @ 2019-02-20 23:32 UTC (permalink / raw)
To: Linux Kernel Mailing List, fsdevel, netdev; +Cc: Luis Chamberlain, Kees Cook
Today, proc_do_large_bitmap() truncates a large write input buffer
to PAGE_SIZE - 1, which may result in misparsed numbers at the
(truncated) end of the buffer. Further, it fails to notify the caller
that the buffer was truncated, so it doesn't get called iteratively
to finish the entire input buffer.
Tell the caller if there's more work to do by adding the skipped
amount back to left/*lenp before returning.
To fix the misparsing, reset the position if we have completely
consumed a truncated buffer (or if just one char is left, which
may be a "-" in a range), and ask the caller to come back for
more.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index ba4d9e85feb8..970a96659809 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -3089,9 +3089,13 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
if (write) {
char *kbuf, *p;
+ size_t skipped = 0;
- if (left > PAGE_SIZE - 1)
+ if (left > PAGE_SIZE - 1) {
left = PAGE_SIZE - 1;
+ /* How much of the buffer we'll skip this pass */
+ skipped = *lenp - left;
+ }
p = kbuf = memdup_user_nul(buffer, left);
if (IS_ERR(kbuf))
@@ -3108,9 +3112,22 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
while (!err && left) {
unsigned long val_a, val_b;
bool neg;
+ size_t saved_left;
+ /* In case we stop parsing mid-number, we can reset */
+ saved_left = left;
err = proc_get_long(&p, &left, &val_a, &neg, tr_a,
sizeof(tr_a), &c);
+ /*
+ * If we consumed the entirety of a truncated buffer or
+ * only one char is left (may be a "-"), then stop here,
+ * reset, & come back for more.
+ */
+ if ((left <= 1) && skipped) {
+ left = saved_left;
+ break;
+ }
+
if (err)
break;
if (val_a >= bitmap_len || neg) {
@@ -3128,6 +3145,15 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
err = proc_get_long(&p, &left, &val_b,
&neg, tr_b, sizeof(tr_b),
&c);
+ /*
+ * If we consumed all of a truncated buffer or
+ * then stop here, reset, & come back for more.
+ */
+ if (!left && skipped) {
+ left = saved_left;
+ break;
+ }
+
if (err)
break;
if (val_b >= bitmap_len || neg ||
@@ -3146,6 +3172,7 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
proc_skip_char(&p, &left, '\n');
}
kfree(kbuf);
+ left += skipped;
} else {
unsigned long bit_a, bit_b = 0;
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers
2019-02-20 23:32 [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers Eric Sandeen
@ 2019-02-20 23:35 ` Eric Sandeen
2019-02-21 15:18 ` Luis Chamberlain
2019-02-21 17:45 ` [PATCH] sysctl: add proc_do_large_bitmap test node Eric Sandeen
2019-03-05 4:43 ` [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers Eric Sandeen
2 siblings, 1 reply; 14+ messages in thread
From: Eric Sandeen @ 2019-02-20 23:35 UTC (permalink / raw)
To: Eric Sandeen, Linux Kernel Mailing List, fsdevel, netdev
Cc: Luis Chamberlain, Kees Cook
Here's a pretty hacky test script to test this code via
ip_local_reserved_ports
-----
#!/bin/bash
# Randomly construct well-formed (sequential, non-overlapping)
# input for ip_local_reserved_ports, feed it to the sysctl,
# then read it back and check for differences.
# Port range to use
PORT_START=1024
PORT_STOP=32768
# Total length of ports string to use
LENGTH=$((4096+$((RANDOM % 16384))))
# String containing our list of ports
PORTS=$PORT_START
# Try 1000 times
for I in `seq 1 1000`; do
# build up the string
while true; do
# Make sure it's discontiguous, skip ahead at least 2
SKIP=$((2 + RANDOM % 10))
PORT_START=$((PORT_START + SKIP))
if [ "$PORT_START" -ge "$PORT_STOP" ]; then
break;
fi
# 14856-14863,14861
# Add a range, or a single port
USERANGE=$((RANDOM % 2))
if [ "$USERANGE" -eq "1" ]; then
RANGE_START=$PORT_START
RANGE_LEN=$((1 + RANDOM % 10))
RANGE_END=$((RANGE_START + RANGE_LEN))
PORTS="${PORTS},${RANGE_START}-${RANGE_END}"
# Break out if we've done enough
if [ "$RANGE_END" -eq "$PORT_STOP" ]; then
break;
fi
PORT_START=$RANGE_END
else
PORTS="${PORTS},${PORT_START}"
fi
if [ "${#PORTS}" -gt "$LENGTH" ]; then
break;
fi
done
# See if we get out what we put in
echo "Trial $I"
echo $PORTS > port_list
cat port_list > /proc/sys/net/ipv4/ip_local_reserved_ports || break
cat /proc/sys/net/ipv4/ip_local_reserved_ports > port_list_out
diff -uq port_list port_list_out || break
done
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers
2019-02-20 23:35 ` Eric Sandeen
@ 2019-02-21 15:18 ` Luis Chamberlain
2019-02-21 17:47 ` Eric Sandeen
0 siblings, 1 reply; 14+ messages in thread
From: Luis Chamberlain @ 2019-02-21 15:18 UTC (permalink / raw)
To: Eric Sandeen, Andrew Morton
Cc: Eric Sandeen, Linux Kernel Mailing List, fsdevel, netdev, Kees Cook
On Wed, Feb 20, 2019 at 05:35:04PM -0600, Eric Sandeen wrote:
> Here's a pretty hacky test script to test this code via
> ip_local_reserved_ports
Thanks Eric!
So /proc/sys/net/ipv4/ip_local_reserved_ports is a production knob, and
if we wanted to stress test it with a selftest it could break other self
tests or change the system behaviour. Because of this we have now have
lib/test_sysctl.c, and we test this with the script:
tools/testing/selftests/sysctl/sysctl.sh
Any chance you can extend lib/test_sysctl.c with a new respective bitmap
knob, and add a respective test? This will ensure we don't regress
later. 0-day runs sysctl.sh so it should catch any regressions in the
future.
If you can think of other ways to test the knob that would be great too.
Luis
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] sysctl: add proc_do_large_bitmap test node
2019-02-20 23:32 [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers Eric Sandeen
2019-02-20 23:35 ` Eric Sandeen
@ 2019-02-21 17:45 ` Eric Sandeen
2019-02-21 18:40 ` Kees Cook
2019-02-21 18:43 ` [PATCH] test_sysctl: add proc_do_large_bitmap test function Eric Sandeen
2019-03-05 4:43 ` [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers Eric Sandeen
2 siblings, 2 replies; 14+ messages in thread
From: Eric Sandeen @ 2019-02-21 17:45 UTC (permalink / raw)
To: Eric Sandeen, Linux Kernel Mailing List, fsdevel, netdev
Cc: Luis Chamberlain, Kees Cook
Add a test node for proc_do_large_bitmap to the test_sysctl.c
infrastructure. It's sized the same as the one existing user.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/lib/test_sysctl.c b/lib/test_sysctl.c
index 3dd801c1c85b..1263be4ebfaf 100644
--- a/lib/test_sysctl.c
+++ b/lib/test_sysctl.c
@@ -47,6 +47,9 @@ struct test_sysctl_data {
unsigned int uint_0001;
char string_0001[65];
+
+#define SYSCTL_TEST_BITMAP_SIZE 65536
+ unsigned long *bitmap_0001;
};
static struct test_sysctl_data test_data = {
@@ -102,6 +106,13 @@ static struct ctl_table test_table[] = {
.mode = 0644,
.proc_handler = proc_dostring,
},
+ {
+ .procname = "bitmap_0001",
+ .data = &test_data.bitmap_0001,
+ .maxlen = SYSCTL_TEST_BITMAP_SIZE,
+ .mode = 0644,
+ .proc_handler = proc_do_large_bitmap,
+ },
{ }
};
@@ -129,15 +140,21 @@ static struct ctl_table_header *test_sysctl_header;
static int __init test_sysctl_init(void)
{
+ test_data.bitmap_0001 = kzalloc(SYSCTL_TEST_BITMAP_SIZE/8, GFP_KERNEL);
+ if (!test_data.bitmap_0001)
+ return -ENOMEM;
test_sysctl_header = register_sysctl_table(test_sysctl_root_table);
- if (!test_sysctl_header)
+ if (!test_sysctl_header) {
+ kfree(test_data.bitmap_0001);
return -ENOMEM;
+ }
return 0;
}
late_initcall(test_sysctl_init);
static void __exit test_sysctl_exit(void)
{
+ kfree(test_data.bitmap_0001);
if (test_sysctl_header)
unregister_sysctl_table(test_sysctl_header);
}
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers
2019-02-21 15:18 ` Luis Chamberlain
@ 2019-02-21 17:47 ` Eric Sandeen
2019-02-21 17:52 ` Luis Chamberlain
0 siblings, 1 reply; 14+ messages in thread
From: Eric Sandeen @ 2019-02-21 17:47 UTC (permalink / raw)
To: Luis Chamberlain, Eric Sandeen, Andrew Morton
Cc: Linux Kernel Mailing List, fsdevel, netdev, Kees Cook
On 2/21/19 9:18 AM, Luis Chamberlain wrote:
> On Wed, Feb 20, 2019 at 05:35:04PM -0600, Eric Sandeen wrote:
>> Here's a pretty hacky test script to test this code via
>> ip_local_reserved_ports
>
> Thanks Eric!
>
> So /proc/sys/net/ipv4/ip_local_reserved_ports is a production knob, and
> if we wanted to stress test it with a selftest it could break other self
> tests or change the system behaviour. Because of this we have now have
> lib/test_sysctl.c, and we test this with the script:
>
> tools/testing/selftests/sysctl/sysctl.sh
>
> Any chance you can extend lib/test_sysctl.c with a new respective bitmap
> knob,
Done
> and add a respective test? This will ensure we don't regress
> later. 0-day runs sysctl.sh so it should catch any regressions in the
> future.
As you know, learning somebody else's test harness infra is a PITA. ;)
Can you find me off-list and give me a hand with this?
Thanks,
-Eric
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers
2019-02-21 17:47 ` Eric Sandeen
@ 2019-02-21 17:52 ` Luis Chamberlain
2019-02-21 17:59 ` Eric Sandeen
0 siblings, 1 reply; 14+ messages in thread
From: Luis Chamberlain @ 2019-02-21 17:52 UTC (permalink / raw)
To: Eric Sandeen
Cc: Eric Sandeen, Andrew Morton, Linux Kernel Mailing List, fsdevel,
netdev, Kees Cook
On Thu, Feb 21, 2019 at 11:47:49AM -0600, Eric Sandeen wrote:
> On 2/21/19 9:18 AM, Luis Chamberlain wrote:
> > On Wed, Feb 20, 2019 at 05:35:04PM -0600, Eric Sandeen wrote:
> >> Here's a pretty hacky test script to test this code via
> >> ip_local_reserved_ports
> >
> > Thanks Eric!
> >
> > So /proc/sys/net/ipv4/ip_local_reserved_ports is a production knob, and
> > if we wanted to stress test it with a selftest it could break other self
> > tests or change the system behaviour. Because of this we have now have
> > lib/test_sysctl.c, and we test this with the script:
> >
> > tools/testing/selftests/sysctl/sysctl.sh
> >
> > Any chance you can extend lib/test_sysctl.c with a new respective bitmap
> > knob,
>
> Done
Thanks!
> > and add a respective test? This will ensure we don't regress
> > later. 0-day runs sysctl.sh so it should catch any regressions in the
> > future.
>
> As you know, learning somebody else's test harness infra is a PITA. ;)
> Can you find me off-list and give me a hand with this?
Sure, its actually quite simple, just as root, run the script.
Luis
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers
2019-02-21 17:52 ` Luis Chamberlain
@ 2019-02-21 17:59 ` Eric Sandeen
0 siblings, 0 replies; 14+ messages in thread
From: Eric Sandeen @ 2019-02-21 17:59 UTC (permalink / raw)
To: Luis Chamberlain, Eric Sandeen
Cc: Andrew Morton, Linux Kernel Mailing List, fsdevel, netdev, Kees Cook
On 2/21/19 11:52 AM, Luis Chamberlain wrote:
> On Thu, Feb 21, 2019 at 11:47:49AM -0600, Eric Sandeen wrote:
>> On 2/21/19 9:18 AM, Luis Chamberlain wrote:
>>> On Wed, Feb 20, 2019 at 05:35:04PM -0600, Eric Sandeen wrote:
>>>> Here's a pretty hacky test script to test this code via
>>>> ip_local_reserved_ports
>>>
>>> Thanks Eric!
>>>
>>> So /proc/sys/net/ipv4/ip_local_reserved_ports is a production knob, and
>>> if we wanted to stress test it with a selftest it could break other self
>>> tests or change the system behaviour. Because of this we have now have
>>> lib/test_sysctl.c, and we test this with the script:
>>>
>>> tools/testing/selftests/sysctl/sysctl.sh
>>>
>>> Any chance you can extend lib/test_sysctl.c with a new respective bitmap
>>> knob,
>>
>> Done
>
> Thanks!
>
>>> and add a respective test? This will ensure we don't regress
>>> later. 0-day runs sysctl.sh so it should catch any regressions in the
>>> future.
>>
>> As you know, learning somebody else's test harness infra is a PITA. ;)
>> Can you find me off-list and give me a hand with this?
>
> Sure, its actually quite simple, just as root, run the script.
Running it looks easy. I'd like to know about how to extend it.
Thanks,
-Eric
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] sysctl: add proc_do_large_bitmap test node
2019-02-21 17:45 ` [PATCH] sysctl: add proc_do_large_bitmap test node Eric Sandeen
@ 2019-02-21 18:40 ` Kees Cook
2019-02-21 18:43 ` [PATCH] test_sysctl: add proc_do_large_bitmap test function Eric Sandeen
1 sibling, 0 replies; 14+ messages in thread
From: Kees Cook @ 2019-02-21 18:40 UTC (permalink / raw)
To: Eric Sandeen
Cc: Eric Sandeen, Linux Kernel Mailing List, fsdevel,
Network Development, Luis Chamberlain
On Thu, Feb 21, 2019 at 9:45 AM Eric Sandeen <sandeen@sandeen.net> wrote:
>
> Add a test node for proc_do_large_bitmap to the test_sysctl.c
> infrastructure. It's sized the same as the one existing user.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Acked-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
>
> diff --git a/lib/test_sysctl.c b/lib/test_sysctl.c
> index 3dd801c1c85b..1263be4ebfaf 100644
> --- a/lib/test_sysctl.c
> +++ b/lib/test_sysctl.c
> @@ -47,6 +47,9 @@ struct test_sysctl_data {
> unsigned int uint_0001;
>
> char string_0001[65];
> +
> +#define SYSCTL_TEST_BITMAP_SIZE 65536
> + unsigned long *bitmap_0001;
> };
>
> static struct test_sysctl_data test_data = {
> @@ -102,6 +106,13 @@ static struct ctl_table test_table[] = {
> .mode = 0644,
> .proc_handler = proc_dostring,
> },
> + {
> + .procname = "bitmap_0001",
> + .data = &test_data.bitmap_0001,
> + .maxlen = SYSCTL_TEST_BITMAP_SIZE,
> + .mode = 0644,
> + .proc_handler = proc_do_large_bitmap,
> + },
> { }
> };
>
> @@ -129,15 +140,21 @@ static struct ctl_table_header *test_sysctl_header;
>
> static int __init test_sysctl_init(void)
> {
> + test_data.bitmap_0001 = kzalloc(SYSCTL_TEST_BITMAP_SIZE/8, GFP_KERNEL);
> + if (!test_data.bitmap_0001)
> + return -ENOMEM;
> test_sysctl_header = register_sysctl_table(test_sysctl_root_table);
> - if (!test_sysctl_header)
> + if (!test_sysctl_header) {
> + kfree(test_data.bitmap_0001);
> return -ENOMEM;
> + }
> return 0;
> }
> late_initcall(test_sysctl_init);
>
> static void __exit test_sysctl_exit(void)
> {
> + kfree(test_data.bitmap_0001);
> if (test_sysctl_header)
> unregister_sysctl_table(test_sysctl_header);
> }
>
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] test_sysctl: add proc_do_large_bitmap test function
2019-02-21 17:45 ` [PATCH] sysctl: add proc_do_large_bitmap test node Eric Sandeen
2019-02-21 18:40 ` Kees Cook
@ 2019-02-21 18:43 ` Eric Sandeen
2019-02-21 19:01 ` Eric Sandeen
2019-02-21 19:16 ` [PATCH V2] " Eric Sandeen
1 sibling, 2 replies; 14+ messages in thread
From: Eric Sandeen @ 2019-02-21 18:43 UTC (permalink / raw)
To: Linux Kernel Mailing List, fsdevel, netdev; +Cc: Luis Chamberlain, Kees Cook
Add test to build up bitmap range string and test the bitmap
proc handler.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
nb: test_modprobe & load_req_mod fail for me before we ever
get to this test, but commenting them out, my test runs as expected.
I'm new to this script, so careful review would be wise. ;)
Thanks,
-Eric
diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh
index 584eb8ea780a..c710b09e2d69 100755
--- a/tools/testing/selftests/sysctl/sysctl.sh
+++ b/tools/testing/selftests/sysctl/sysctl.sh
@@ -37,6 +37,7 @@ ALL_TESTS="$ALL_TESTS 0002:1:1"
ALL_TESTS="$ALL_TESTS 0003:1:1"
ALL_TESTS="$ALL_TESTS 0004:1:1"
ALL_TESTS="$ALL_TESTS 0005:3:1"
+ALL_TESTS="$ALL_TESTS 0006:3:1"
test_modprobe()
{
@@ -149,6 +150,9 @@ reset_vals()
string_0001)
VAL="(none)"
;;
+ bitmap_0001)
+ VAL=""
+ ;;
*)
;;
esac
@@ -548,6 +552,47 @@ run_stringtests()
test_rc
}
+run_bitmaptest() {
+ # Total length of bitmaps string to use, a bit under
+ # the maximum input size of the test node
+ LENGTH=$((RANDOM % 65000))
+
+ # First bit to set
+ BIT=$((RANDOM % 1024))
+
+ # String containing our list of bits to set
+ TEST_STR=$BIT
+
+ # build up the string
+ while [ "${#TEST_STR}" -le "$LENGTH" ]; do
+ # Make sure next entry is discontiguous,
+ # skip ahead at least 2
+ BIT=$((BIT + $((2 + RANDOM % 10))))
+
+ # Add new bit to the list
+ TEST_STR="${TEST_STR},${BIT}"
+
+ # Randomly make it a range
+ if [ "$((RANDOM % 2))" -eq "1" ]; then
+ RANGE_END=$((BIT + $((1 + RANDOM % 10))))
+ TEST_STR="${TEST_STR}-${RANGE_END}"
+ BIT=$RANGE_END
+ fi
+ done
+
+ echo -n "Checking bitmap handler... "
+ set_orig
+ echo -n $TEST_STR > $TARGET 2> /dev/null
+
+ if verify "${TARGET}"; then
+ echo "FAIL" >&2
+ rc=1
+ else
+ echo "ok"
+ fi
+ test_rc
+}
+
sysctl_test_0001()
{
TARGET="${SYSCTL}/int_0001"
@@ -605,6 +650,14 @@ sysctl_test_0005()
run_limit_digit_int_array
}
+sysctl_test_0006()
+{
+ TARGET="${SYSCTL}/bitmap_0001"
+ reset_vals
+ ORIG=$(cat "${TARGET}")
+ run_bitmaptest
+}
+
list_tests()
{
echo "Test ID list:"
@@ -618,6 +671,7 @@ list_tests()
echo "0003 x $(get_test_count 0003) - tests proc_dointvec()"
echo "0004 x $(get_test_count 0004) - tests proc_douintvec()"
echo "0005 x $(get_test_count 0005) - tests proc_douintvec() array"
+ echo "0006 x $(get_test_count 0006) - tests proc_do_large_bitmap"
}
test_reqs
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] test_sysctl: add proc_do_large_bitmap test function
2019-02-21 18:43 ` [PATCH] test_sysctl: add proc_do_large_bitmap test function Eric Sandeen
@ 2019-02-21 19:01 ` Eric Sandeen
2019-02-21 19:16 ` [PATCH V2] " Eric Sandeen
1 sibling, 0 replies; 14+ messages in thread
From: Eric Sandeen @ 2019-02-21 19:01 UTC (permalink / raw)
To: Eric Sandeen, Linux Kernel Mailing List, fsdevel, netdev
Cc: Luis Chamberlain, Kees Cook
On 2/21/19 12:43 PM, Eric Sandeen wrote:
> Add test to build up bitmap range string and test the bitmap
> proc handler.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> nb: test_modprobe & load_req_mod fail for me before we ever
> get to this test, but commenting them out, my test runs as expected.
> I'm new to this script, so careful review would be wise. ;)
>
> Thanks,
> -Eric
Gah, running this in different ways, it's not doing the right thing.
Hang on, V2 pending...
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V2] test_sysctl: add proc_do_large_bitmap test function
2019-02-21 18:43 ` [PATCH] test_sysctl: add proc_do_large_bitmap test function Eric Sandeen
2019-02-21 19:01 ` Eric Sandeen
@ 2019-02-21 19:16 ` Eric Sandeen
1 sibling, 0 replies; 14+ messages in thread
From: Eric Sandeen @ 2019-02-21 19:16 UTC (permalink / raw)
To: Eric Sandeen, Linux Kernel Mailing List, fsdevel, netdev
Cc: Luis Chamberlain, Kees Cook
Add test to build up bitmap range string and test the bitmap
proc handler.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
V2: set rc=0 for test success
however this still fails indeterminately for me. Debugging, if I
save off the test write string and the read string, re-writing it to
the handler works fine. My standalone test also works fine for 1000
iterations. So I don't know what's going on here.
nb: test_modprobe & load_req_mod fail for me before we ever
get to this test, but commenting them out, my test runs as expected.
I'm new to this script, so careful review would be wise.
Thanks,
-Eric
diff --git a/tools/testing/selftests/sysctl/sysctl.sh b/tools/testing/selftests/sysctl/sysctl.sh
index 584eb8ea780a..e531074f94bd 100755
--- a/tools/testing/selftests/sysctl/sysctl.sh
+++ b/tools/testing/selftests/sysctl/sysctl.sh
@@ -37,6 +37,7 @@ ALL_TESTS="$ALL_TESTS 0002:1:1"
ALL_TESTS="$ALL_TESTS 0003:1:1"
ALL_TESTS="$ALL_TESTS 0004:1:1"
ALL_TESTS="$ALL_TESTS 0005:3:1"
+ALL_TESTS="$ALL_TESTS 0006:3:1"
test_modprobe()
{
@@ -149,6 +150,9 @@ reset_vals()
string_0001)
VAL="(none)"
;;
+ bitmap_0001)
+ VAL=""
+ ;;
*)
;;
esac
@@ -548,6 +552,48 @@ run_stringtests()
test_rc
}
+run_bitmaptest() {
+ # Total length of bitmaps string to use, a bit under
+ # the maximum input size of the test node
+ LENGTH=$((RANDOM % 65000))
+
+ # First bit to set
+ BIT=$((RANDOM % 1024))
+
+ # String containing our list of bits to set
+ TEST_STR=$BIT
+
+ # build up the string
+ while [ "${#TEST_STR}" -le "$LENGTH" ]; do
+ # Make sure next entry is discontiguous,
+ # skip ahead at least 2
+ BIT=$((BIT + $((2 + RANDOM % 10))))
+
+ # Add new bit to the list
+ TEST_STR="${TEST_STR},${BIT}"
+
+ # Randomly make it a range
+ if [ "$((RANDOM % 2))" -eq "1" ]; then
+ RANGE_END=$((BIT + $((1 + RANDOM % 10))))
+ TEST_STR="${TEST_STR}-${RANGE_END}"
+ BIT=$RANGE_END
+ fi
+ done
+
+ echo -n "Checking bitmap handler... "
+ echo $TEST_STR > /tmp/test_str
+ echo -n $TEST_STR > $TARGET 2> /dev/null
+
+ if verify "${TARGET}"; then
+ echo "FAIL" >&2
+ rc=1
+ else
+ echo "ok"
+ rc=0
+ fi
+ test_rc
+}
+
sysctl_test_0001()
{
TARGET="${SYSCTL}/int_0001"
@@ -605,6 +651,14 @@ sysctl_test_0005()
run_limit_digit_int_array
}
+sysctl_test_0006()
+{
+ TARGET="${SYSCTL}/bitmap_0001"
+ reset_vals
+ ORIG=""
+ run_bitmaptest
+}
+
list_tests()
{
echo "Test ID list:"
@@ -618,6 +672,7 @@ list_tests()
echo "0003 x $(get_test_count 0003) - tests proc_dointvec()"
echo "0004 x $(get_test_count 0004) - tests proc_douintvec()"
echo "0005 x $(get_test_count 0005) - tests proc_douintvec() array"
+ echo "0006 x $(get_test_count 0006) - tests proc_do_large_bitmap"
}
test_reqs
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers
2019-02-20 23:32 [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers Eric Sandeen
2019-02-20 23:35 ` Eric Sandeen
2019-02-21 17:45 ` [PATCH] sysctl: add proc_do_large_bitmap test node Eric Sandeen
@ 2019-03-05 4:43 ` Eric Sandeen
2019-03-19 15:30 ` Luis Chamberlain
2 siblings, 1 reply; 14+ messages in thread
From: Eric Sandeen @ 2019-03-05 4:43 UTC (permalink / raw)
To: Eric Sandeen, Linux Kernel Mailing List, fsdevel, netdev
Cc: Luis Chamberlain, Kees Cook
On 2/20/19 5:32 PM, Eric Sandeen wrote:
> Today, proc_do_large_bitmap() truncates a large write input buffer
> to PAGE_SIZE - 1, which may result in misparsed numbers at the
> (truncated) end of the buffer. Further, it fails to notify the caller
> that the buffer was truncated, so it doesn't get called iteratively
> to finish the entire input buffer.
>
> Tell the caller if there's more work to do by adding the skipped
> amount back to left/*lenp before returning.
>
> To fix the misparsing, reset the position if we have completely
> consumed a truncated buffer (or if just one char is left, which
> may be a "-" in a range), and ask the caller to come back for
> more.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Would be nice to fix this bug. I submitted the test node patch as well
as an attempt to integrate it into the test harness, though there's
wonkiness there still, and I could use more experienced eyes.
Can we move this forward?
Thanks,
-Eric
> ---
>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index ba4d9e85feb8..970a96659809 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -3089,9 +3089,13 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
>
> if (write) {
> char *kbuf, *p;
> + size_t skipped = 0;
>
> - if (left > PAGE_SIZE - 1)
> + if (left > PAGE_SIZE - 1) {
> left = PAGE_SIZE - 1;
> + /* How much of the buffer we'll skip this pass */
> + skipped = *lenp - left;
> + }
>
> p = kbuf = memdup_user_nul(buffer, left);
> if (IS_ERR(kbuf))
> @@ -3108,9 +3112,22 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
> while (!err && left) {
> unsigned long val_a, val_b;
> bool neg;
> + size_t saved_left;
>
> + /* In case we stop parsing mid-number, we can reset */
> + saved_left = left;
> err = proc_get_long(&p, &left, &val_a, &neg, tr_a,
> sizeof(tr_a), &c);
> + /*
> + * If we consumed the entirety of a truncated buffer or
> + * only one char is left (may be a "-"), then stop here,
> + * reset, & come back for more.
> + */
> + if ((left <= 1) && skipped) {
> + left = saved_left;
> + break;
> + }
> +
> if (err)
> break;
> if (val_a >= bitmap_len || neg) {
> @@ -3128,6 +3145,15 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
> err = proc_get_long(&p, &left, &val_b,
> &neg, tr_b, sizeof(tr_b),
> &c);
> + /*
> + * If we consumed all of a truncated buffer or
> + * then stop here, reset, & come back for more.
> + */
> + if (!left && skipped) {
> + left = saved_left;
> + break;
> + }
> +
> if (err)
> break;
> if (val_b >= bitmap_len || neg ||
> @@ -3146,6 +3172,7 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
> proc_skip_char(&p, &left, '\n');
> }
> kfree(kbuf);
> + left += skipped;
> } else {
> unsigned long bit_a, bit_b = 0;
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers
2019-03-05 4:43 ` [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers Eric Sandeen
@ 2019-03-19 15:30 ` Luis Chamberlain
2019-03-19 15:57 ` Luis Chamberlain
0 siblings, 1 reply; 14+ messages in thread
From: Luis Chamberlain @ 2019-03-19 15:30 UTC (permalink / raw)
To: Eric Sandeen
Cc: Eric Sandeen, Linux Kernel Mailing List, fsdevel, netdev, Kees Cook
On Mon, Mar 04, 2019 at 10:43:09PM -0600, Eric Sandeen wrote:
> On 2/20/19 5:32 PM, Eric Sandeen wrote:
> > Today, proc_do_large_bitmap() truncates a large write input buffer
> > to PAGE_SIZE - 1, which may result in misparsed numbers at the
> > (truncated) end of the buffer. Further, it fails to notify the caller
> > that the buffer was truncated, so it doesn't get called iteratively
> > to finish the entire input buffer.
> >
> > Tell the caller if there's more work to do by adding the skipped
> > amount back to left/*lenp before returning.
> >
> > To fix the misparsing, reset the position if we have completely
> > consumed a truncated buffer (or if just one char is left, which
> > may be a "-" in a range), and ask the caller to come back for
> > more.
> >
> > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>
> Would be nice to fix this bug. I submitted the test node patch as well
> as an attempt to integrate it into the test harness, though there's
> wonkiness there still, and I could use more experienced eyes.
Sorry for the delay.
I'm rolling these changes in with some minor adjustments, can you send
me your respective lib/test_sysctl.c changes? I don't see that they had
been sent.
Luis
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers
2019-03-19 15:30 ` Luis Chamberlain
@ 2019-03-19 15:57 ` Luis Chamberlain
0 siblings, 0 replies; 14+ messages in thread
From: Luis Chamberlain @ 2019-03-19 15:57 UTC (permalink / raw)
To: Eric Sandeen
Cc: Eric Sandeen, Linux Kernel Mailing List, fsdevel, netdev, Kees Cook
On Tue, Mar 19, 2019 at 9:30 AM Luis Chamberlain <mcgrof@kernel.org> wrote:
> can you send
> me your respective lib/test_sysctl.c changes? I don't see that they had
> been sent.
Never mind, I see it now, will roll all this in.
Luis
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-03-19 16:04 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-20 23:32 [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers Eric Sandeen
2019-02-20 23:35 ` Eric Sandeen
2019-02-21 15:18 ` Luis Chamberlain
2019-02-21 17:47 ` Eric Sandeen
2019-02-21 17:52 ` Luis Chamberlain
2019-02-21 17:59 ` Eric Sandeen
2019-02-21 17:45 ` [PATCH] sysctl: add proc_do_large_bitmap test node Eric Sandeen
2019-02-21 18:40 ` Kees Cook
2019-02-21 18:43 ` [PATCH] test_sysctl: add proc_do_large_bitmap test function Eric Sandeen
2019-02-21 19:01 ` Eric Sandeen
2019-02-21 19:16 ` [PATCH V2] " Eric Sandeen
2019-03-05 4:43 ` [PATCH] sysctl: Fix proc_do_large_bitmap for large input buffers Eric Sandeen
2019-03-19 15:30 ` Luis Chamberlain
2019-03-19 15:57 ` Luis Chamberlain
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.