On Mon, Jun 12, 2023 at 10:58:39PM +0200, Pali Rohár wrote: > On Monday 12 June 2023 23:52:30 Andy Shevchenko wrote: > > On Mon, Jun 12, 2023 at 07:52:05PM +0200, Pali Rohár wrote: > > > On Monday 12 June 2023 12:02:50 Michal Wilczynski wrote: ... > > > Hello! I'm looking at rbtn_add() function and there is also code: > > > > > > rbtn_data = devm_kzalloc(&device->dev, sizeof(*rbtn_data), GFP_KERNEL); > > > if (!rbtn_data) > > > return -ENOMEM; > > > > > > which is called after rbtn_acquire(). So it looks like when kzalloc > > > fails then there is another leak... > > > > Side note: In that case we would need a devm wrapper on acquire call. > > Does it makes sense to invest time and more resources for these fixes? > Driver is not used on new Dell machines, so I would not expect new > users (instead less users, if they start upgrading HW to new Dell > machines). > > Simple fix for this issue: Just move devm_kzalloc() call before > rbtn_acquire(true). And call cleanup rbtn_acquire(false) exactly like > Michal did in this patch. > > I think that this should be enough, should cover all failure paths and > does not require to introduce new code or new design, which should be > properly tested for no regression. > > What do you think? Sounds like a good alternative! Thank you for the review. -- With Best Regards, Andy Shevchenko
