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 <sand...@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;
>
>
>
