lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAFR=A7nkyx_Lf=p0BS-S68_vxQL97rUoLMZpo4kxHjKykAgTRw@mail.gmail.com> Date: Sat, 18 Nov 2023 14:27:56 +0100 From: Kamil Duljas <kamil.duljas@...il.com> To: Florian Westphal <fw@...len.de> Cc: Jakub Kicinski <kuba@...nel.org>, "David S . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Jiri Pirko <jiri@...nulli.us>, Johannes Berg <johannes@...solutions.net>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH] genetlink: Prevent memory leak when krealloc fail Yes, you're right. I did not think about it. So if we have a static pointer that may be resued, should not restore the pointer as at the beginning? static unsigned long *mc_groups = &mc_group_start; At this moment we don't know how much memory is allocated. What do you think about this? > new_groups = krealloc(mc_groups, nlen, > GFP_KERNEL); > - if (!new_groups) > + if (!new_groups) { > + kfree(mc_groups); > + mc_groups = &mc_group_start; > return -ENOMEM; > + } sob., 18 lis 2023 o 13:02 Florian Westphal <fw@...len.de> napisaĆ(a): > > Kamil Duljas <kamil.duljas@...il.com> wrote: > > genl_allocate_reserve_groups() allocs new memory in while loop > > but if krealloc fail, the memory allocated by kzalloc is not freed. > > It seems allocated memory is unnecessary when the function > > returns -ENOMEM > > Why should it be free'd? mc_groups is not a local variable. > > > new_groups = krealloc(mc_groups, nlen, > > GFP_KERNEL); > > - if (!new_groups) > > + if (!new_groups) { > > + kfree(mc_groups); > > return -ENOMEM; > > + } > > How did you test this? AFAICS this results in use-after-free for every > access to mc_groups after this error path is taken. > > Existing code looks correct, we can't grow mc_groups and return an > error. -- Pozdrawiam, Kamil Duljas
Powered by blists - more mailing lists