Re: [PATCH net] net: genl: fix error path memory leak in policy dumping

[Jakub Kicinski <kuba@kernel.org>]

On Mon, 15 Aug 2022 18:08:58 -0300 Rafael Soares wrote:
> On Mon, Aug 15, 2022 at 11:20:21AM -0700, Jakub Kicinski wrote:
> > If construction of the array of policies fails when recording
> > non-first policy we need to unwind.
> > 
> > Reported-by: syzbot+dc54d9ba8153b216cae0@syzkaller.appspotmail.com
> > Fixes: 50a896cf2d6f ("genetlink: properly support per-op policy dumping")
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > ---
> >  net/netlink/genetlink.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
> > index 1afca2a6c2ac..57010927e20a 100644
> > --- a/net/netlink/genetlink.c
> > +++ b/net/netlink/genetlink.c
> > @@ -1174,13 +1174,17 @@ static int ctrl_dumppolicy_start(struct netlink_callback *cb)
> >  							     op.policy,
> >  							     op.maxattr);
> >  			if (err)
> > -				return err;
> > +				goto err_free_state;  
> 
> There's another call to netlink_policy_dump_add_policy() right above
> this one. The patch I posted to syzkaller frees the memory inside
> netlink_policy_dump_add_policy() and the result was OK.

Oh, sorry, I didn't see this reply. Every single time after the merge
window is over the stable folks bombard the mailing systems with their
backported patches and bring the mailing lists to their knees :/

Do you see your posting on lore.kernel.org? I presume not. You really
need to use ./scripts/get_maintainer and CC relevant folks directly.

This is v2 of my fix I submitted to syzbot for testing:

https://git.kernel.org/pub/scm/linux/kernel/git/kuba/linux.git/commit/?h=genl-fix&id=66f01a660c4439fc78a6fc68413f895b8fd07474

The earlier call to netlink_policy_dump_add_policy() should not be 
a problem if the function unwound its state properly.