linux-kernel - Re: [PATCH 3/3] kernfs: Convert kernfs_path_from_node

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231117084757.150724ed@rorschach.local.home>
Date:   Fri, 17 Nov 2023 08:47:57 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Tejun Heo <tj@...nel.org>, Zefan Li <lizefan.x@...edance.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Waiman Long <longman@...hat.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        cgroups@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
        Azeem Shaikh <azeemshaikh38@...il.com>,
        linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
        linux-hardening@...r.kernel.org
Subject: Re: [PATCH 3/3] kernfs: Convert kernfs_path_from_node_locked() from
 strlcpy() to strscpy()

On Thu, 16 Nov 2023 11:21:25 -0800
Kees Cook <keescook@...omium.org> 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@...uxfoundation.org>
> Cc: Tejun Heo <tj@...nel.org>
> Cc: Zefan Li <lizefan.x@...edance.com>
> Cc: Johannes Weiner <hannes@...xchg.org>
> Cc: Waiman Long <longman@...hat.com>
> Cc: Steven Rostedt <rostedt@...dmis.org>
> Cc: Masami Hiramatsu <mhiramat@...nel.org>
> Cc: cgroups@...r.kernel.org
> Cc: linux-trace-kernel@...r.kernel.org
> Co-developed-by: Azeem Shaikh <azeemshaikh38@...il.com>
> Signed-off-by: Azeem Shaikh <azeemshaikh38@...il.com>
> Signed-off-by: Kees Cook <keescook@...omium.org>
> ---
>  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