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