Re: "cat /proc/sys/kernel/random/entropy_avail" sometimes causes page allocation failure

[Vlastimil Babka <vbabka@suse.cz>]

On 2/1/21 2:52 PM, Matthew Wilcox wrote:
> On Mon, Feb 01, 2021 at 01:20:04PM +0100, Vlastimil Babka wrote:
>> On 2/1/21 12:26 PM, Steven Noonan wrote:
>> > (Please CC me on replies, I'm not subscribed to the list.)
>> > 
>> > I started seeing this problem several months ago, but after some Google searches I concluded that someone had already root caused and fixed it, and I just needed to wait for a newer kernel to come along:
>> > 
>> > https://www.spinics.net/lists/linux-mm/msg226311.html
>> 
>> Yeah, that thread mentions Josef's series [1], but I don't see that it made it
>> into git log. What happened to it? Meanwhile Matthew refactored it with
>> "4bd6a7353ee1 ("sysctl: Convert to iter interfaces")" which left the kzalloc()
>> but changed count to count+1, which would explain the change from order-5 to
>> order-6. In my quick test, strace cat tells me it uses 128k sized read, which is
>> order-5.
>> 
>> So miminally there should be kvzalloc(), but it's still somewhat unfortunate to
>> do that for reading a few bytes.
>> Also the check for KMALLOC_MAX_SIZE happens before the +1. Meh. Matthew?
> 
> That's why the check is >= KMALLOC_MAX_SIZE.
> 
> Switching to kvzalloc() means we'll only allocate one extra page, not twice
> as many pages.  We can't know how large the proc file is going to want the
> buffer to be, but perhaps we can do this ...

Looks ok to me, but I'm not that familiar with iov_iter stuff. Care to send a
proper patch?

> @@ -569,17 +569,16 @@ static ssize_t proc_sys_call_handler(struct kiocb *iocb, struct iov_iter *iter,
>  
>         /* don't even try if the size is too large */
>         error = -ENOMEM;
> -       if (count >= KMALLOC_MAX_SIZE)
> +       if (count > KMALLOC_MAX_SIZE)
>                 goto out;
> -       kbuf = kzalloc(count + 1, GFP_KERNEL);
> +       kbuf = kvzalloc(count, GFP_KERNEL);
>         if (!kbuf)
>                 goto out;
>  
>         if (write) {
>                 error = -EFAULT;
> -               if (!copy_from_iter_full(kbuf, count, iter))
> +               if (!copy_from_iter_full(kbuf, count - 1, iter))
>                         goto out_free_buf;
> -               kbuf[count] = '\0';
>         }
>  
>         error = BPF_CGROUP_RUN_PROG_SYSCTL(head, table, write, &kbuf, &count,
> 
>