| Message ID | 20240609092937.2874379-1-joe@pf.is.s.u-tokyo.ac.jp (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | char: tpm: Fix possible memory leak in tpm_bios_measurements_open() | expand |
On Sun Jun 9, 2024 at 12:29 PM EEST, Joe Hattori wrote: > In tpm_bios_measurements_open(), get_device() is called on the device > embedded in struct tpm_chip. In the error path, however, put_device() is > not called. This could result in a reference count leak, which could When it does not then? > prevent the device from being properly released. This commit makes sure > to call put_device() when the tpm_bios_measurements_open() fails. s/could//g Either *is* or *is not*. > > Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp> > --- > drivers/char/tpm/eventlog/common.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/char/tpm/eventlog/common.c b/drivers/char/tpm/eventlog/common.c > index 639c3f395a5a..df213ec428ca 100644 > --- a/drivers/char/tpm/eventlog/common.c > +++ b/drivers/char/tpm/eventlog/common.c > @@ -44,11 +44,13 @@ static int tpm_bios_measurements_open(struct inode *inode, > > /* now register seq file */ > err = seq_open(file, seqops); > - if (!err) { > - seq = file->private_data; > - seq->private = chip; > + if (err) { > + put_device(&chip->dev); > + return err; > } > > + seq = file->private_data; > + seq->private = chip; So this does two things: 1. It restructures code. 2. Possibly fixes a leak (with quick skimo does). So it breaks "Separate your changes" section of [1]. Instead: if (!err) { seq = file->private_data; seq->private = chip; } else { put_device(&chip->dev); } I.e. least likelyhood of merge conflicts, when backporting. Also, add a fixes tag (also [1]). [1] https://docs.kernel.org/process/submitting-patches.html BR, Jarkko
diff --git a/drivers/char/tpm/eventlog/common.c b/drivers/char/tpm/eventlog/common.c index 639c3f395a5a..df213ec428ca 100644 --- a/drivers/char/tpm/eventlog/common.c +++ b/drivers/char/tpm/eventlog/common.c @@ -44,11 +44,13 @@ static int tpm_bios_measurements_open(struct inode *inode, /* now register seq file */ err = seq_open(file, seqops); - if (!err) { - seq = file->private_data; - seq->private = chip; + if (err) { + put_device(&chip->dev); + return err; } + seq = file->private_data; + seq->private = chip; return err; }
In tpm_bios_measurements_open(), get_device() is called on the device embedded in struct tpm_chip. In the error path, however, put_device() is not called. This could result in a reference count leak, which could prevent the device from being properly released. This commit makes sure to call put_device() when the tpm_bios_measurements_open() fails. Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp> --- drivers/char/tpm/eventlog/common.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)