| Message ID | 20220916235205.106873-1-russell.h.weight@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/1] fpga: m10bmc-sec: Fix possible memory leak of flash_buf | expand |
On 9/16/22 4:52 PM, Russ Weight wrote: > There is an error check following the allocation of flash_buf that returns > without freeing flash_buf. It makes more sense to do the error check > before the allocation and the reordering eliminates the memory leak. > > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Fixes: 154afa5c31cd ("fpga: m10bmc-sec: expose max10 flash update count") > Signed-off-by: Russ Weight <russell.h.weight@intel.com> Reviewed-by: Tom Rix <trix@redhat.com> > Cc: <stable@vger.kernel.org> > --- > drivers/fpga/intel-m10-bmc-sec-update.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c > index 526c8cdd1474..79d48852825e 100644 > --- a/drivers/fpga/intel-m10-bmc-sec-update.c > +++ b/drivers/fpga/intel-m10-bmc-sec-update.c > @@ -148,10 +148,6 @@ static ssize_t flash_count_show(struct device *dev, > stride = regmap_get_reg_stride(sec->m10bmc->regmap); > num_bits = FLASH_COUNT_SIZE * 8; > > - flash_buf = kmalloc(FLASH_COUNT_SIZE, GFP_KERNEL); > - if (!flash_buf) > - return -ENOMEM; > - > if (FLASH_COUNT_SIZE % stride) { > dev_err(sec->dev, > "FLASH_COUNT_SIZE (0x%x) not aligned to stride (0x%x)\n", > @@ -160,6 +156,10 @@ static ssize_t flash_count_show(struct device *dev, > return -EINVAL; > } > > + flash_buf = kmalloc(FLASH_COUNT_SIZE, GFP_KERNEL); > + if (!flash_buf) > + return -ENOMEM; > + > ret = regmap_bulk_read(sec->m10bmc->regmap, STAGING_FLASH_COUNT, > flash_buf, FLASH_COUNT_SIZE / stride); > if (ret) {
On 2022-09-16 at 17:20:43 -0700, Tom Rix wrote: > > On 9/16/22 4:52 PM, Russ Weight wrote: > > There is an error check following the allocation of flash_buf that returns > > without freeing flash_buf. It makes more sense to do the error check > > before the allocation and the reordering eliminates the memory leak. > > > > Reported-by: kernel test robot <lkp@intel.com> > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > Fixes: 154afa5c31cd ("fpga: m10bmc-sec: expose max10 flash update count") > > Signed-off-by: Russ Weight <russell.h.weight@intel.com> > Reviewed-by: Tom Rix <trix@redhat.com> Acked-by: Xu Yilun <yilun.xu@intel.com> Applied to for-6.0 > > Cc: <stable@vger.kernel.org> > > --- > > drivers/fpga/intel-m10-bmc-sec-update.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c > > index 526c8cdd1474..79d48852825e 100644 > > --- a/drivers/fpga/intel-m10-bmc-sec-update.c > > +++ b/drivers/fpga/intel-m10-bmc-sec-update.c > > @@ -148,10 +148,6 @@ static ssize_t flash_count_show(struct device *dev, > > stride = regmap_get_reg_stride(sec->m10bmc->regmap); > > num_bits = FLASH_COUNT_SIZE * 8; > > - flash_buf = kmalloc(FLASH_COUNT_SIZE, GFP_KERNEL); > > - if (!flash_buf) > > - return -ENOMEM; > > - > > if (FLASH_COUNT_SIZE % stride) { > > dev_err(sec->dev, > > "FLASH_COUNT_SIZE (0x%x) not aligned to stride (0x%x)\n", > > @@ -160,6 +156,10 @@ static ssize_t flash_count_show(struct device *dev, > > return -EINVAL; > > } > > + flash_buf = kmalloc(FLASH_COUNT_SIZE, GFP_KERNEL); > > + if (!flash_buf) > > + return -ENOMEM; > > + > > ret = regmap_bulk_read(sec->m10bmc->regmap, STAGING_FLASH_COUNT, > > flash_buf, FLASH_COUNT_SIZE / stride); > > if (ret) { >
diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c index 526c8cdd1474..79d48852825e 100644 --- a/drivers/fpga/intel-m10-bmc-sec-update.c +++ b/drivers/fpga/intel-m10-bmc-sec-update.c @@ -148,10 +148,6 @@ static ssize_t flash_count_show(struct device *dev, stride = regmap_get_reg_stride(sec->m10bmc->regmap); num_bits = FLASH_COUNT_SIZE * 8; - flash_buf = kmalloc(FLASH_COUNT_SIZE, GFP_KERNEL); - if (!flash_buf) - return -ENOMEM; - if (FLASH_COUNT_SIZE % stride) { dev_err(sec->dev, "FLASH_COUNT_SIZE (0x%x) not aligned to stride (0x%x)\n", @@ -160,6 +156,10 @@ static ssize_t flash_count_show(struct device *dev, return -EINVAL; } + flash_buf = kmalloc(FLASH_COUNT_SIZE, GFP_KERNEL); + if (!flash_buf) + return -ENOMEM; + ret = regmap_bulk_read(sec->m10bmc->regmap, STAGING_FLASH_COUNT, flash_buf, FLASH_COUNT_SIZE / stride); if (ret) {
There is an error check following the allocation of flash_buf that returns without freeing flash_buf. It makes more sense to do the error check before the allocation and the reordering eliminates the memory leak. Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Fixes: 154afa5c31cd ("fpga: m10bmc-sec: expose max10 flash update count") Signed-off-by: Russ Weight <russell.h.weight@intel.com> Cc: <stable@vger.kernel.org> --- drivers/fpga/intel-m10-bmc-sec-update.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)