Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] proc: split __free(kvfree) declaration and initialization in pde_write_buf() --
commit d672761b59bb5333b9344e6a78cc97135a5b70b1 Author: Christian Brauner <[email protected]> proc: let procfs copy in writes before pinning the entry This commit introduces ->proc_write_buf() in struct proc_ops so procfs can copy user write buffers into a NUL-terminated kernel buffer before pinning the proc_dir_entry, preventing page faults during user copies from blocking remove_proc_entry(). > diff --git a/fs/proc/inode.c b/fs/proc/inode.c > index 8425465aae5d7..c0bb40e7ff964 100644 > --- a/fs/proc/inode.c > +++ b/fs/proc/inode.c > @@ -440,11 +440,44 @@ static ssize_t pde_write(struct proc_dir_entry *pde, > struct file *file, const ch > return -EIO; > } > > +/* > + * procfs copies in what is written before it pins the entry. A writer that > + * faults on its buffer then doesn't hold up remove_proc_entry(). > + */ > +static ssize_t pde_write_buf(struct proc_dir_entry *pde, struct file *file, > + const char __user *ubuf, size_t count, loff_t > *ppos) > +{ > + char *buf __free(kvfree) = NULL; > + size_t size; > + ssize_t rv; > + > + size = pde->write_size; > + if (count > size) { > + if (!pde_has_short_writes(pde)) > + return -EINVAL; > + count = size; > + } > + buf = kvmalloc(count + 1, GFP_KERNEL); [Severity: Low] This isn't a bug, but in pde_write_buf() (called via proc_reg_write()), buf is declared with __free(kvfree) = NULL at the top of the function and assigned later after the count > size check. Per the Cleanup API conventions, could buf be defined and initialized in a single statement at the point of allocation: char *buf __free(kvfree) = kvmalloc(count + 1, GFP_KERNEL); after the size check? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
