Use of memset() should be systematic on stack-allocated structures for security reasons. The compiler normally eliminates the dead stores.

That said, here, it's unclear to me there is a problem in the first place: All fields are effectively filled, they are all 4-byte aligned, including the fields with enumeration type (the underlying type being int), and the total structure size is a multiple of 8, so there's no padding anywhere. Which are the bytes that are actually leaked?

In D59309#1360722, @olce wrote:

Use of memset() should be systematic on stack-allocated structures for security reasons. The compiler normally eliminates the dead stores.

That said, here, it's unclear to me there is a problem in the first place: All fields are effectively filled, they are all 4-byte aligned, including the fields with enumeration type (the underlying type being int), and the total structure size is a multiple of 8, so there's no padding anywhere. Which are the bytes that are actually leaked?

Bytes trailing the nul terminator in the device_name field are leaked.

In D59309#1360723, @markj wrote:

Bytes trailing the nul terminator in the device_name field are leaked.

Oh, the strcpy()... Perhaps the strcpy() could also be replaced by a memcpy() with sizeof(...device_name), does not hurt to have it along with the added memset(), it's a better example for people coding such kind of conversions.

In D59309#1360722, @olce wrote:

Use of memset() should be systematic on stack-allocated structures for security reasons. The compiler normally eliminates the dead stores.

Sure. One simple test which might help catch such bugs would be to run sysctl -a or equivalent from a 32-bit executable running under KMSAN. Even better would be if we could compile a 32-bit userland and boot and test it with a 64-bit kernel.

In D59309#1360730, @olce wrote:

In D59309#1360723, @markj wrote:

Bytes trailing the nul terminator in the device_name field are leaked.

Oh, the strcpy()... Perhaps the strcpy() could also be replaced by a memcpy() with sizeof(...device_name), does not hurt to have it along with the added memset(), it's a better example for people coding such kind of conversions.

Well, we should really just use memset() everywhere. We have fixed many similar bugs over the years and adding plain memset(&<struct>, 0, sizeof(struct)) has been the common solution.

strncpy() would also work since it fills the full buffer, but it has other downsides.

In D59309#1360831, @kib wrote:

You could memset only device_name then, right before strcpy(). It would be more clear IMHO.

In general we have fixed these bugs by zeroing the whole struct, so I somewhat prefer to keep following that pattern. It will automatically catch problems if new fields are added to struct devstat; note that there have been several revisions to the struct over time.