Re: [PATCH bpf-next 1/1] bpf: fix possible file descriptor leaks in verifier

[Anton Protopopov <aspsk@isovalent.com>]

On Thu, Mar 28, 2024 at 08:23:03PM -0700, Yonghong Song wrote:
> 
> On 3/28/24 2:56 AM, Anton Protopopov wrote:
> > The resolve_pseudo_ldimm64() function might have leaked file
> > descriptors when BPF_MAP_TYPE_ARENA was used in a program (some
> > error paths missed a corresponding fdput).
> > 
> > Fix leaks and also extract code which adds maps to env->used_maps
> > into a separate function. This simplifies code of resolve_pseudo_ldimm64
> > and makes it possible to reuse this code later.  While at it, also add a
> > verifier verbose message when the maps usage limit is reached.
> 
> Could you break this single patch into a series? For example,
> for 'to reuse this code later', refactoring the code (without
> funcitonality change) can be a separate patch, could you explain
> what do you mean reusing the code later? Do you have some plans
> to reuse the code later?

Yes, to pre-process file descriptors in attrs.fd_array (or a similar array).
See this thread https://lore.kernel.org/bpf/ZgWcsKZR23JIg51e@lavr/

> The fix for missing fdput() should be its own patch with
> Fixes tag below.
> 
> You can have yet another patch to enhance verifier by adding
> verbose messages.

Thanks. I will just send two fixes for fdput and the verifier message.
I will send a patch which refactors code when it is needed.

> > 
> > Fixes: 6082b6c328b5 ("bpf: Recognize addr_space_cast instruction in the verifier.")
> > Signed-off-by: Anton Protopopov <aspsk@isovalent.com>
> > ---
> >   kernel/bpf/verifier.c | 150 ++++++++++++++++++++++--------------------
> >   1 file changed, 79 insertions(+), 71 deletions(-)
> [...]