Hi Andi, Anyway we cannot keep strcpy, if name is not NULL terminated case, msg.name is overflowed. Trying to find some safe design pattern about that, I've found strscpy: https://lwn.net/Articles/643376/ https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=30c44659f4a3e7e1f9f47e895591b4b40bf62671
which clearly indicates error in case of overflow, so code becomes: - memcpy(msg.name, name, sizeof(msg.name)); - msg.name[sizeof(msg.name) - 1] = 0; + if (strscpy(msg.name, name, sizeof(msg.name)) <= 0) + goto err; What do you think of this proposal ? Best regards, Hugues. On 01/04/2018 01:19 AM, Andi Kleen wrote: > On Wed, Jan 03, 2018 at 09:40:04AM +0000, Hugues FRUCHET wrote: >> Hi Andi, >> Thanks for the patch but I would suggest to use strlcpy instead, this >> will guard msg.name overwriting and add the NULL termination in case >> of truncation: >> - memcpy(msg.name, name, sizeof(msg.name)); >> - msg.name[sizeof(msg.name) - 1] = 0; >> + strlcpy(msg.name, name, sizeof(msg.name)); > > I'm not an expert on your setup, but it seems strlcpy would leak some > uninitialized stack data over your ipc mechanism. strclpy doesn't pad the > data. If the IPC is a security boundary that would be a security bug. > > So I think the original patch is better than strlcpy. > > -Andi >
