Re: [PATCH v2 1/2] ovl: port to new mount api

["Colin Walters" <walters@verbum.org>]

Cool work.  It will be interesting to do some performance testing on what does it actually look like to create ~500 or whatever overlayfs layers now that we can.

On Fri, Jun 9, 2023, at 11:41 AM, Christian Brauner wrote:
> 
> +static int ovl_init_fs_context(struct fs_context *fc)
> +{
> +	struct ovl_fs_context *ctx = NULL;
> +	struct ovl_fs *ofs = NULL;
> +
> +	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL_ACCOUNT);
> +	if (!ctx)
> +		goto out_err;

It looks to me like in this case, ofs will be NULL, then:

> +out_err:
> +	ovl_fs_context_free(ctx);
> +	ovl_free_fs(ofs);

And then we'll jump here and `ovl_free_fs` is not NULL safe.

I think the previous code was correct here as it just jumped directly to "out:".


(I've always wondered why there's no usage of __attribute__((cleanup)) in kernel code and in our userspace code doing that we have the free functions be no-ops on NULL which systematically avoids these bugs, but then again maybe the real fix is Rust ;) )