Re: [syzbot] [fs?] possible deadlock in ovl_create_object (2)

[Chris Roy]

On Sat, Sep 19, 2026, Jörn Engel wrote:
> Finding good names is probably a fetish of mine, so "val" is mildly
> disturbing to me. [...]

I have a similar pet peeve in my domain. I kept "val" since
it mirrors block2mtd_setup2()'s own "val" parameter
(the raw <dev>[,[<erasesize>][,<label>]]), so I kept it for
consistency with the existing call chain. I had considered
"spec" but probably it would have been similarly unimaginative.

> Is this check necessary? [...] Looks like the kernel is still
> undecided whether checks are necessary or not.

I will keep it. kzalloc()/kstrdup() under plain GFP_KERNEL can still
return NULL under real memory pressure (no __GFP_NOFAIL here), and
dropping it would be inconsistent with the kstrdup() check two lines
below.

> That's quite a few calls to the same function. [...] If you feel
> motivated to dig deeper, please do! If not, I'd rather leave the
> mess in place than come up with a half-hearted attempt of a
> solution.

True. It is accretion. I'd rather not bolt a redesign onto this fix.
However, I can look at setup2()'s callers as a separate effort.

Regards,
- Chris

"But how could you live and have no story to tell?"

On Sat, 19 Sept 2026 at 20:16, Jörn Engel <joern@xxxxxxxxxxxxxxxx> wrote:
>
> On Sat, Sep 19, 2026 at 02:22:15PM +0530, Chris Roy wrote:
> >
> > +struct block2mtd_setup_work {
> > +     struct work_struct work;
> > +     struct completion done;
> > +     char *val;
> > +     int ret;
> > +};
>
> Finding good names is probably a fetish of mine, so "val" is mildly
> disturbing to me.  It's not an objectively bad name, just something
> where I'd spend another five minutes trying to come up with something
> better.
>
> > +/* Runs block2mtd_setup2() on setup_wq, blocking until it completes */
> > +static int block2mtd_setup_defer(const char *val)
> > +{
> > +     struct block2mtd_setup_work *w;
> > +     int ret;
> > +
> > +     w = kzalloc(sizeof(*w), GFP_KERNEL);
> > +     if (!w)
> > +             return -ENOMEM;
>
> Is this check necessary?  I'm in the camp of "malloc should never return
> NULL".  That condition is so rare that it is effectively impossible to
> trust callers with error handling.  So the right approach is to crash
> (or kernel panic here) instead of returning an error.
>
> Looking into sources I see this:
> static inline void *kzalloc(size_t s, gfp_t gfp)
> > {
> >       void *p = kmalloc(s, gfp);
> >
> >       memset(p, 0, s);
> >       return p;
> > }
>
> We have at least one example of explicitly not checking the return
> value.  But we have other prominent examples of checking as well.  Looks
> like the kernel is still undecided whether checks are necessary or not.
>
> > -     return block2mtd_setup2(val);
> ...
> > -             return block2mtd_setup2(val);
> ...
> > +             ret = block2mtd_setup2(val);
> ...
> >               ret = block2mtd_setup2(block2mtd_paramline);
>
> That's quite a few calls to the same function.  If setup gets that
> complicated, that's a strong indication that we're doing things wrong.
> Typically we get into such a mess one well-intentioned change at a time.
> Find a bug, fix it by adding another caller, repeat.
>
> It isn't obvious how we could improve things.  Sometimes it's lack of
> infrastructure and the entire complicated setup-dance should be moved to
> common code.  Driver can then call a single function to deal with
> everything.
>
> But if there's only a single driver using such infrastructure, maybe the
> driver is doing things wrong and should copy whatever other drivers are
> doing.  The problem shouldn't be unique to a single driver, so we should
> find the best solution and then use it everywhere.
>
> I don't know the correct answer here.  If you feel motivated to dig
> deeper, please do!  If not, I'd rather leave the mess in place than come
> up with a half-hearted attempt of a solution.
>
> Jörn
>
> --
> You cannot suppose that Moliere ever troubled himself to be original in the
> matter of ideas. You cannot suppose that the stories he tells in his plays
> have never been told before. They were culled, as you very well know.
> -- Andre-Louis Moreau in Scarabouche