| Message ID | 20210222151231.22572-1-romain.perier@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Manual replacement of all strlcpy in favor of strscpy | expand |
On 2/22/21 8:12 AM, Romain Perier wrote: > strlcpy() copy a C-String into a sized buffer, the result is always a > valid NULL-terminated that fits in the buffer, howerver it has severals > issues. It reads the source buffer first, which is dangerous if it is non > NULL-terminated or if the corresponding buffer is unbounded. Its safe > replacement is strscpy(), as suggested in the deprecated interface [1]. > > We plan to make this contribution in two steps: > - Firsly all cases of strlcpy's return value are manually replaced by the > corresponding calls of strscpy() with the new handling of the return > value (as the return code is different in case of error). > - Then all other cases are automatically replaced by using coccinelle. > Cool. A quick check shows me 1031 strscpy() calls with no return checks. All or some of these probably need to be reviewed and add return checks. Is this something that is in the plan to address as part of this work? thanks, -- Shuah
Le lun. 22 févr. 2021 à 17:36, Shuah Khan <skhan@linuxfoundation.org> a écrit : > > Cool. A quick check shows me 1031 strscpy() calls with no return > checks. All or some of these probably need to be reviewed and add > return checks. Is this something that is in the plan to address as > part of this work? > > thanks, > -- Shuah > Hi, Initially, what we planned with Kees is to firstly replace all calls with error handling codes (like this series does), and then replace all other simple calls (without error handling). However, we can also start a discussion about this topic, all suggestions are welcome. I am not sure that it does make sense to check all returns code in all cases (for example in arch/alpha/kernel/setup.c, there are a ton of other examples in the kernel). But a general review (as you suggest), would make sense. Regards, Romain
strlcpy() copy a C-String into a sized buffer, the result is always a valid NULL-terminated that fits in the buffer, howerver it has severals issues. It reads the source buffer first, which is dangerous if it is non NULL-terminated or if the corresponding buffer is unbounded. Its safe replacement is strscpy(), as suggested in the deprecated interface [1]. We plan to make this contribution in two steps: - Firsly all cases of strlcpy's return value are manually replaced by the corresponding calls of strscpy() with the new handling of the return value (as the return code is different in case of error). - Then all other cases are automatically replaced by using coccinelle. This series covers manual replacements. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy Romain Perier (20): cgroup: Manual replacement of the deprecated strlcpy() with return values crypto: Manual replacement of the deprecated strlcpy() with return values devlink: Manual replacement of the deprecated strlcpy() with return values dma-buf: Manual replacement of the deprecated strlcpy() with return values kobject: Manual replacement of the deprecated strlcpy() with return values ima: Manual replacement of the deprecated strlcpy() with return values SUNRPC: Manual replacement of the deprecated strlcpy() with return values kernfs: Manual replacement of the deprecated strlcpy() with return values m68k/atari: Manual replacement of the deprecated strlcpy() with return values module: Manual replacement of the deprecated strlcpy() with return values hwmon: Manual replacement of the deprecated strlcpy() with return values s390/hmcdrv: Manual replacement of the deprecated strlcpy() with return values scsi: zfcp: Manual replacement of the deprecated strlcpy() with return values target: Manual replacement of the deprecated strlcpy() with return values ALSA: usb-audio: Manual replacement of the deprecated strlcpy() with return values tracing/probe: Manual replacement of the deprecated strlcpy() with return values vt: Manual replacement of the deprecated strlcpy() with return values usb: gadget: f_midi: Manual replacement of the deprecated strlcpy() with return values usbip: usbip_host: Manual replacement of the deprecated strlcpy() with return values s390/watchdog: Manual replacement of the deprecated strlcpy() with return values arch/m68k/emu/natfeat.c | 6 +-- crypto/lrw.c | 6 +-- crypto/xts.c | 6 +-- drivers/dma-buf/dma-buf.c | 4 +- drivers/hwmon/pmbus/max20730.c | 66 +++++++++++++------------ drivers/s390/char/diag_ftp.c | 4 +- drivers/s390/char/sclp_ftp.c | 6 +-- drivers/s390/scsi/zfcp_fc.c | 8 +-- drivers/target/target_core_configfs.c | 33 ++++--------- drivers/tty/vt/keyboard.c | 5 +- drivers/usb/gadget/function/f_midi.c | 4 +- drivers/usb/gadget/function/f_printer.c | 8 +-- drivers/usb/usbip/stub_main.c | 6 +-- drivers/watchdog/diag288_wdt.c | 12 +++-- fs/kernfs/dir.c | 27 +++++----- kernel/cgroup/cgroup.c | 2 +- kernel/module.c | 4 +- kernel/trace/trace_uprobe.c | 11 ++--- lib/kobject_uevent.c | 6 +-- net/core/devlink.c | 6 +-- net/sunrpc/clnt.c | 6 ++- security/integrity/ima/ima_policy.c | 8 ++- sound/usb/card.c | 4 +- 23 files changed, 129 insertions(+), 119 deletions(-)