netdev - Re: [RFC 0/3] bpf: Add d

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]

Date:   Thu, 2 Apr 2020 15:21:06 +0100
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Jiri Olsa <jolsa@...nel.org>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
        bpf@...r.kernel.org, Yonghong Song <yhs@...com>,
        Martin KaFai Lau <kafai@...com>,
        David Miller <davem@...hat.com>,
        John Fastabend <john.fastabend@...il.com>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        Wenbo Zhang <ethercflow@...il.com>,
        KP Singh <kpsingh@...omium.org>,
        Andrii Nakryiko <andriin@...com>, bgregg@...flix.com
Subject: Re: [RFC 0/3] bpf: Add d_path helper

On Wed, Apr 01, 2020 at 01:09:04PM +0200, Jiri Olsa wrote:
> hi,
> adding d_path helper to return full path for 'path' object.
> 
> I originally added and used 'file_path' helper, which did the same,
> but used 'struct file' object. Then realized that file_path is just
> a wrapper for d_path, so we'd cover more calling sites if we add
> d_path helper and allowed resolving BTF object within another object,
> so we could call d_path also with file pointer, like:
> 
>   bpf_d_path(&file->f_path, buf, size);
> 
> This feature is mainly to be able to add dpath (filepath originally)
> function to bpftrace, which seems to work nicely now, like:
> 
>   # bpftrace -e 'kretfunc:fget { printf("%s\n", dpath(args->ret->f_path));  }' 
> 
> I'm not completely sure this is all safe and bullet proof and there's
> no other way to do this, hence RFC post.
> 
> I'd be happy also with file_path function, but I thought it'd be
> a shame not to try to add d_path with the verifier change.
> I'm open to any suggestions ;-)

What are the locking conditions guaranteed to that sucker?  Note that d_path()
is *NOT* lockless - call it from an interrupt/NMI/etc. and you are fucked.
It can grab rename_lock and mount_lock; usually it avoids that, so you won't
see them grabbed on every call, but after the first seqlock mismatch it will
fall back to grabbing the spinlock in question.  And then there's ->d_dname(),
with whatever things _that_ chooses to do....