Re: [PATCH 3/3] kernfs: Convert kernfs_path_from_node_locked() from strlcpy() to strscpy()

[Steven Rostedt]

On Thu, 16 Nov 2023 11:21:25 -0800
Kees Cook <keescook@xxxxxxxxxxxx> wrote:

> One of the last remaining users of strlcpy() in the kernel is
> kernfs_path_from_node_locked(), which passes back the problematic "length
> we _would_ have copied" return value to indicate truncation.  Convert the
> chain of all callers to use the negative return value (some of which
> already doing this explicitly). All callers were already also checking
> for negative return values, so the risk to missed checks looks very low.
> 
> In this analysis, it was found that cgroup1_release_agent() actually
> didn't handle the "too large" condition, so this is technically also a
> bug fix. :)
> 
> Here's the chain of callers, and resolution identifying each one as now
> handling the correct return value:
> 
> kernfs_path_from_node_locked()
>         kernfs_path_from_node()
>                 pr_cont_kernfs_path()
>                         returns void
>                 kernfs_path()
>                         sysfs_warn_dup()
>                                 return value ignored
>                         cgroup_path()
>                                 blkg_path()
>                                         bfq_bic_update_cgroup()
>                                                 return value ignored
>                                 TRACE_IOCG_PATH()
>                                         return value ignored
>                                 TRACE_CGROUP_PATH()
>                                         return value ignored
>                                 perf_event_cgroup()
>                                         return value ignored
>                                 task_group_path()
>                                         return value ignored
>                                 damon_sysfs_memcg_path_eq()
>                                         return value ignored
>                                 get_mm_memcg_path()
>                                         return value ignored
>                                 lru_gen_seq_show()
>                                         return value ignored
>                         cgroup_path_from_kernfs_id()
>                                 return value ignored
>                 cgroup_show_path()
>                         already converted "too large" error to negative value
>                 cgroup_path_ns_locked()
>                         cgroup_path_ns()
>                                 bpf_iter_cgroup_show_fdinfo()
>                                         return value ignored
>                                 cgroup1_release_agent()
>                                         wasn't checking "too large" error
>                         proc_cgroup_show()
>                                 already converted "too large" to negative value
> 
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Tejun Heo <tj@xxxxxxxxxx>
> Cc: Zefan Li <lizefan.x@xxxxxxxxxxxxx>
> Cc: Johannes Weiner <hannes@xxxxxxxxxxx>
> Cc: Waiman Long <longman@xxxxxxxxxx>
> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Cc: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> Cc: cgroups@xxxxxxxxxxxxxxx
> Cc: linux-trace-kernel@xxxxxxxxxxxxxxx
> Co-developed-by: Azeem Shaikh <azeemshaikh38@xxxxxxxxx>
> Signed-off-by: Azeem Shaikh <azeemshaikh38@xxxxxxxxx>
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> ---
>  fs/kernfs/dir.c             | 37 ++++++++++++++++++++-----------------
>  kernel/cgroup/cgroup-v1.c   |  2 +-
>  kernel/cgroup/cgroup.c      |  4 ++--
>  kernel/cgroup/cpuset.c      |  2 +-
>  kernel/trace/trace_uprobe.c |  2 +-

trace_uprobe.c seems out of scope for this patch.

-- Steve