Re: [PATCH v2] kernfs: Prefer strscpy over strlcpy calls

[Tejun Heo <tj@kernel.org>]

Hello,

On Wed, May 10, 2023 at 12:03:41PM -0400, Azeem Shaikh wrote:
> > >  static int kernfs_name_locked(struct kernfs_node *kn, char *buf, size_t buflen)
> > >  {
> > >       if (!kn)
> > > -             return strlcpy(buf, "(null)", buflen);
> > > +             return strscpy_mock_strlcpy(buf, "(null)", buflen);
> > >
> > > -     return strlcpy(buf, kn->parent ? kn->name : "/", buflen);
> > > +     return strscpy_mock_strlcpy(buf, kn->parent ? kn->name : "/", buflen);
> > >  }
> >
> > Can you follow the users and convert the users accordingly rather than
> > masking it from here? ie. make kernfs_name() and friends return -E2BIG when
> > source is too long like strscpy(). I don't think anybody cares, actually.
> >
> 
> I found 4 transitive callers of kernfs_name across the kernel, all of
> whom eventually ignored the return value:
> 
> 1. fs/kernfs/dir.c: calls kernfs_name. Ignores return value.
> 2. include/linux/cgroup.h: calls kernfs_name from cgroup_name. returns
> the value of kernfs_name.
> 3. kernel/cgroup/debug.c: calls cgroup_name. Ignores return value.
> 4.mm/page_owner.c: calls cgroup_name. Ignores return value.
> 
> So replacing directly with strscpy here should be safe. Let me know
> what you think.

That sounds great to me. I have a hard time imagining needing the length
return for single component name.

> > >  /* kernfs_node_depth - compute depth from @from to @to */
> > > @@ -141,13 +148,13 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
...
> > Ditto, please convert all the users accordingly. If that's not feasible, I
> > think it'd be better to leave it as-is. I don't see how the new code is
> > better.
> 
> kernfs_path_from_node has quite a few transitive callers. I'll leave
> this as-is for now and consider tackling this separately.

Yeah, I could be misremembering but istr some place which actually uses the
length return to extend buffer allocation, so this might be challenging.

Thanks.

-- 
tejun