Hi Bjorn, [...] > > + if (count >= (PAGE_SIZE - 1)) > > + return -EINVAL; > > I'm not the sysfs expert, but surely the sysfs infrastructure already > guarantees this? We don't need to store any value, since we are processing the input from the userspace, thus ensuring that we have room for the newline is not needed, especially since the show() function dynamically builds the content to show, so indeed this check can be dropped. To add, there aren't any guarantees other from sysfs than we get a up to a PAGE_SIZE worth of data in the buffer. [...] > > + options = kstrndup(buf, count, GFP_KERNEL); > > I assume the kstrndup() is because strsep() writes into the buffer? Yes, Amey added kstrndup() in v6 following my recommendation as per: https://lore.kernel.org/linux-pci/20210606125800.GA76573@rocinante.localdomain/ This was to avoid removing the const quantifier through a type cast given that the signature of the function denotes that the buffer is a pointer to immutable string, as per: https://elixir.bootlin.com/linux/v5.14-rc3/source/include/linux/device/driver.h#L137 Some other sysfs users do employ the cast when using strtok() and I am not so such it's the right way to do it, as per: drivers/s390/net/qeth_l3_sys.c 232: tmp = strsep((char **)&buf, "\n"); drivers/media/rc/rc-main.c 1167: while ((tmp = strsep((char **)&buf, " \n")) != NULL) { > Aren't we allowed to write into the buffer we get from sysfs? Does > the user ever see the buffer contents again? I would think sysfs > would have already done a copy_from_user() or whatever. I might be wrong about this, but I suppose this might be to stop people from accidentally freeing the buffer as kernfs_fop_write_iter() would do it after all the internal housekeeping is done, provided that someone pays attention to compile time warnings. Krzysztof
