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
| ||
|
Date: Thu, 30 Jul 2020 12:22:52 +0200 From: Jiri Olsa <jolsa@...hat.com> To: Al Viro <viro@...iv.linux.org.uk> Cc: Jiri Olsa <jolsa@...nel.org>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andriin@...com>, netdev@...r.kernel.org, bpf@...r.kernel.org, Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>, Martin KaFai Lau <kafai@...com>, David Miller <davem@...hat.com>, John Fastabend <john.fastabend@...il.com>, Wenbo Zhang <ethercflow@...il.com>, KP Singh <kpsingh@...omium.org>, Brendan Gregg <bgregg@...flix.com>, Florent Revest <revest@...omium.org> Subject: Re: [PATCH v8 bpf-next 09/13] bpf: Add d_path helper On Wed, Jul 29, 2020 at 09:11:17PM +0100, Al Viro wrote: > On Wed, Jul 22, 2020 at 11:12:19PM +0200, Jiri Olsa wrote: > > > +BPF_CALL_3(bpf_d_path, struct path *, path, char *, buf, u32, sz) > > +{ > > + char *p = d_path(path, buf, sz - 1); > > + int len; > > + > > + if (IS_ERR(p)) { > > + len = PTR_ERR(p); > > + } else { > > + len = strlen(p); > > + if (len && p != buf) > > + memmove(buf, p, len); > > *blink* > What the hell do you need that strlen() for? d_path() copies into > the end of buffer (well, starts there and prepends to it); all you > really need is memmove(buf, p, buf + sz - p) I used the code from some of the other users like backing_dev_show fsg_show_file nice, looks like we could omit strlen call in perf mmap event call as well > > > > + buf[len] = 0; > > Wait a minute... Why are you NUL-terminating it separately? > You do rely upon having NUL in the damn thing (and d_path() does > guarantee it there). Without that strlen() would've gone into > the nasal demon country; you can't call it on non-NUL-terminated > array. So you are guaranteed that p[len] will be '\0'; why bother > copying the first len bytes and then separately deal with that > NUL? Just memmove() the fucker and be done with that... > > If you are worried about stray NUL in the middle of the returned > data... can't happen. Note the rename_lock use in fs/d_path.c; > the names of everything involved are guaranteed to have been > stable throughout the copying them into the buffer - if anything > were to be renamed while we are doing that, we'd repeat the whole > thing (with rename_lock taken exclusive the second time around). > > So make it simply > if (IS_ERR(p)) > return PTR_ERR(p); > len = buf + sz - p; > memmove(buf, p, len); > return len; ok, will use this > and be done with that. BTW, the odds of p == buf are pretty much > nil - it would happen only if sz - 1 happened to be the exact length > of pathname. > ok, great thanks, jirka
Powered by blists - more mailing lists