On Thu, Apr 10, 2025 at 09:51:09AM +0200, Stefano Garzarella wrote: > On Thu, Apr 10, 2025 at 09:14:58AM +0300, Jarkko Sakkinen wrote: > > On Thu, Apr 10, 2025 at 02:25:36AM +0530, Purva Yeshi wrote: > > > Fix Smatch-detected error:
Empty line and s/error/issue/. > > > drivers/char/tpm/tpm-buf.c:208 tpm_buf_read_u8() error: > > > uninitialized symbol 'value'. > > > drivers/char/tpm/tpm-buf.c:225 tpm_buf_read_u16() error: > > > uninitialized symbol 'value'. > > > drivers/char/tpm/tpm-buf.c:242 tpm_buf_read_u32() error: > > > uninitialized symbol 'value'. > > > > > > Call tpm_buf_read() to populate value but do not check its return > > > status. If the read fails, value remains uninitialized, causing > > > undefined behavior when returned or processed. > > > > > > Initialize value to zero to ensure a defined return even if > > > tpm_buf_read() fails, avoiding undefined behavior from using > > > an uninitialized variable. > > > > How does tpm_buf_read() fail? > > If TPM_BUF_BOUNDARY_ERROR is set (or we are setting it), we are effectively > returning random stack bytes to the caller. > Could this be a problem? It should never happen, if the kernel is working correctly. The commit message implies a legit failure scenario, which would imply that the patch is a bug fix, which it actually is not. "Add a sanity check for boundary overflow, and zero out the value, if the unexpected happens" is what this patch actually does. I.e., code change acceptable, commit message is all wrong. > > If it is, maybe instead of this patch, we could set `*output` to zero in the > error path of tpm_buf_read(). Or return an error from tpm_buf_read() so > callers can return 0 or whatever they want. > > Thanks, > Stefano > > BR, Jarkko
