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, > >
