Revision 57f0ff059e3daa4e70a811cb1d31a49968262d20 authored by Michael Petlan on 19 July 2021, 14:53:32 UTC, committed by Arnaldo Carvalho de Melo on 18 September 2021, 20:43:05 UTC
It's later supposed to be either a correct address or NULL. Without the initialization, it may contain an undefined value which results in the following segmentation fault: # perf top --sort comm -g --ignore-callees=do_idle terminates with: #0 0x00007ffff56b7685 in __strlen_avx2 () from /lib64/libc.so.6 #1 0x00007ffff55e3802 in strdup () from /lib64/libc.so.6 #2 0x00005555558cb139 in hist_entry__init (callchain_size=<optimized out>, sample_self=true, template=0x7fffde7fb110, he=0x7fffd801c250) at util/hist.c:489 #3 hist_entry__new (template=template@entry=0x7fffde7fb110, sample_self=sample_self@entry=true) at util/hist.c:564 #4 0x00005555558cb4ba in hists__findnew_entry (hists=hists@entry=0x5555561d9e38, entry=entry@entry=0x7fffde7fb110, al=al@entry=0x7fffde7fb420, sample_self=sample_self@entry=true) at util/hist.c:657 #5 0x00005555558cba1b in __hists__add_entry (hists=hists@entry=0x5555561d9e38, al=0x7fffde7fb420, sym_parent=<optimized out>, bi=bi@entry=0x0, mi=mi@entry=0x0, sample=sample@entry=0x7fffde7fb4b0, sample_self=true, ops=0x0, block_info=0x0) at util/hist.c:288 #6 0x00005555558cbb70 in hists__add_entry (sample_self=true, sample=0x7fffde7fb4b0, mi=0x0, bi=0x0, sym_parent=<optimized out>, al=<optimized out>, hists=0x5555561d9e38) at util/hist.c:1056 #7 iter_add_single_cumulative_entry (iter=0x7fffde7fb460, al=<optimized out>) at util/hist.c:1056 #8 0x00005555558cc8a4 in hist_entry_iter__add (iter=iter@entry=0x7fffde7fb460, al=al@entry=0x7fffde7fb420, max_stack_depth=<optimized out>, arg=arg@entry=0x7fffffff7db0) at util/hist.c:1231 #9 0x00005555557cdc9a in perf_event__process_sample (machine=<optimized out>, sample=0x7fffde7fb4b0, evsel=<optimized out>, event=<optimized out>, tool=0x7fffffff7db0) at builtin-top.c:842 #10 deliver_event (qe=<optimized out>, qevent=<optimized out>) at builtin-top.c:1202 #11 0x00005555558a9318 in do_flush (show_progress=false, oe=0x7fffffff80e0) at util/ordered-events.c:244 #12 __ordered_events__flush (oe=oe@entry=0x7fffffff80e0, how=how@entry=OE_FLUSH__TOP, timestamp=timestamp@entry=0) at util/ordered-events.c:323 #13 0x00005555558a9789 in __ordered_events__flush (timestamp=<optimized out>, how=<optimized out>, oe=<optimized out>) at util/ordered-events.c:339 #14 ordered_events__flush (how=OE_FLUSH__TOP, oe=0x7fffffff80e0) at util/ordered-events.c:341 #15 ordered_events__flush (oe=oe@entry=0x7fffffff80e0, how=how@entry=OE_FLUSH__TOP) at util/ordered-events.c:339 #16 0x00005555557cd631 in process_thread (arg=0x7fffffff7db0) at builtin-top.c:1114 #17 0x00007ffff7bb817a in start_thread () from /lib64/libpthread.so.0 #18 0x00007ffff5656dc3 in clone () from /lib64/libc.so.6 If you look at the frame #2, the code is: 488 if (he->srcline) { 489 he->srcline = strdup(he->srcline); 490 if (he->srcline == NULL) 491 goto err_rawdata; 492 } If he->srcline is not NULL (it is not NULL if it is uninitialized rubbish), it gets strdupped and strdupping a rubbish random string causes the problem. Also, if you look at the commit 1fb7d06a509e, it adds the srcline property into the struct, but not initializing it everywhere needed. Committer notes: Now I see, when using --ignore-callees=do_idle we end up here at line 2189 in add_callchain_ip(): 2181 if (al.sym != NULL) { 2182 if (perf_hpp_list.parent && !*parent && 2183 symbol__match_regex(al.sym, &parent_regex)) 2184 *parent = al.sym; 2185 else if (have_ignore_callees && root_al && 2186 symbol__match_regex(al.sym, &ignore_callees_regex)) { 2187 /* Treat this symbol as the root, 2188 forgetting its callees. */ 2189 *root_al = al; 2190 callchain_cursor_reset(cursor); 2191 } 2192 } And the al that doesn't have the ->srcline field initialized will be copied to the root_al, so then, back to: 1211 int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al, 1212 int max_stack_depth, void *arg) 1213 { 1214 int err, err2; 1215 struct map *alm = NULL; 1216 1217 if (al) 1218 alm = map__get(al->map); 1219 1220 err = sample__resolve_callchain(iter->sample, &callchain_cursor, &iter->parent, 1221 iter->evsel, al, max_stack_depth); 1222 if (err) { 1223 map__put(alm); 1224 return err; 1225 } 1226 1227 err = iter->ops->prepare_entry(iter, al); 1228 if (err) 1229 goto out; 1230 1231 err = iter->ops->add_single_entry(iter, al); 1232 if (err) 1233 goto out; 1234 That al at line 1221 is what hist_entry_iter__add() (called from sample__resolve_callchain()) saw as 'root_al', and then: iter->ops->add_single_entry(iter, al); will go on with al->srcline with a bogus value, I'll add the above sequence to the cset and apply, thanks! Signed-off-by: Michael Petlan <mpetlan@redhat.com> CC: Milian Wolff <milian.wolff@kdab.com> Cc: Jiri Olsa <jolsa@redhat.com> Fixes: 1fb7d06a509e ("perf report Use srcline from callchain for hist entries") Link: https //lore.kernel.org/r/20210719145332.29747-1-mpetlan@redhat.com Reported-by: Juri Lelli <jlelli@redhat.com> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
1 parent ff6f41f
d_path.c
/* SPDX-License-Identifier: GPL-2.0 */
#include <linux/syscalls.h>
#include <linux/export.h>
#include <linux/uaccess.h>
#include <linux/fs_struct.h>
#include <linux/fs.h>
#include <linux/slab.h>
#include <linux/prefetch.h>
#include "mount.h"
struct prepend_buffer {
char *buf;
int len;
};
#define DECLARE_BUFFER(__name, __buf, __len) \
struct prepend_buffer __name = {.buf = __buf + __len, .len = __len}
static char *extract_string(struct prepend_buffer *p)
{
if (likely(p->len >= 0))
return p->buf;
return ERR_PTR(-ENAMETOOLONG);
}
static bool prepend_char(struct prepend_buffer *p, unsigned char c)
{
if (likely(p->len > 0)) {
p->len--;
*--p->buf = c;
return true;
}
p->len = -1;
return false;
}
/*
* The source of the prepend data can be an optimistoc load
* of a dentry name and length. And because we don't hold any
* locks, the length and the pointer to the name may not be
* in sync if a concurrent rename happens, and the kernel
* copy might fault as a result.
*
* The end result will correct itself when we check the
* rename sequence count, but we need to be able to handle
* the fault gracefully.
*/
static bool prepend_copy(void *dst, const void *src, int len)
{
if (unlikely(copy_from_kernel_nofault(dst, src, len))) {
memset(dst, 'x', len);
return false;
}
return true;
}
static bool prepend(struct prepend_buffer *p, const char *str, int namelen)
{
// Already overflowed?
if (p->len < 0)
return false;
// Will overflow?
if (p->len < namelen) {
// Fill as much as possible from the end of the name
str += namelen - p->len;
p->buf -= p->len;
prepend_copy(p->buf, str, p->len);
p->len = -1;
return false;
}
// Fits fully
p->len -= namelen;
p->buf -= namelen;
return prepend_copy(p->buf, str, namelen);
}
/**
* prepend_name - prepend a pathname in front of current buffer pointer
* @buffer: buffer pointer
* @buflen: allocated length of the buffer
* @name: name string and length qstr structure
*
* With RCU path tracing, it may race with d_move(). Use READ_ONCE() to
* make sure that either the old or the new name pointer and length are
* fetched. However, there may be mismatch between length and pointer.
* But since the length cannot be trusted, we need to copy the name very
* carefully when doing the prepend_copy(). It also prepends "/" at
* the beginning of the name. The sequence number check at the caller will
* retry it again when a d_move() does happen. So any garbage in the buffer
* due to mismatched pointer and length will be discarded.
*
* Load acquire is needed to make sure that we see the new name data even
* if we might get the length wrong.
*/
static bool prepend_name(struct prepend_buffer *p, const struct qstr *name)
{
const char *dname = smp_load_acquire(&name->name); /* ^^^ */
u32 dlen = READ_ONCE(name->len);
return prepend(p, dname, dlen) && prepend_char(p, '/');
}
static int __prepend_path(const struct dentry *dentry, const struct mount *mnt,
const struct path *root, struct prepend_buffer *p)
{
while (dentry != root->dentry || &mnt->mnt != root->mnt) {
const struct dentry *parent = READ_ONCE(dentry->d_parent);
if (dentry == mnt->mnt.mnt_root) {
struct mount *m = READ_ONCE(mnt->mnt_parent);
struct mnt_namespace *mnt_ns;
if (likely(mnt != m)) {
dentry = READ_ONCE(mnt->mnt_mountpoint);
mnt = m;
continue;
}
/* Global root */
mnt_ns = READ_ONCE(mnt->mnt_ns);
/* open-coded is_mounted() to use local mnt_ns */
if (!IS_ERR_OR_NULL(mnt_ns) && !is_anon_ns(mnt_ns))
return 1; // absolute root
else
return 2; // detached or not attached yet
}
if (unlikely(dentry == parent))
/* Escaped? */
return 3;
prefetch(parent);
if (!prepend_name(p, &dentry->d_name))
break;
dentry = parent;
}
return 0;
}
/**
* prepend_path - Prepend path string to a buffer
* @path: the dentry/vfsmount to report
* @root: root vfsmnt/dentry
* @buffer: pointer to the end of the buffer
* @buflen: pointer to buffer length
*
* The function will first try to write out the pathname without taking any
* lock other than the RCU read lock to make sure that dentries won't go away.
* It only checks the sequence number of the global rename_lock as any change
* in the dentry's d_seq will be preceded by changes in the rename_lock
* sequence number. If the sequence number had been changed, it will restart
* the whole pathname back-tracing sequence again by taking the rename_lock.
* In this case, there is no need to take the RCU read lock as the recursive
* parent pointer references will keep the dentry chain alive as long as no
* rename operation is performed.
*/
static int prepend_path(const struct path *path,
const struct path *root,
struct prepend_buffer *p)
{
unsigned seq, m_seq = 0;
struct prepend_buffer b;
int error;
rcu_read_lock();
restart_mnt:
read_seqbegin_or_lock(&mount_lock, &m_seq);
seq = 0;
rcu_read_lock();
restart:
b = *p;
read_seqbegin_or_lock(&rename_lock, &seq);
error = __prepend_path(path->dentry, real_mount(path->mnt), root, &b);
if (!(seq & 1))
rcu_read_unlock();
if (need_seqretry(&rename_lock, seq)) {
seq = 1;
goto restart;
}
done_seqretry(&rename_lock, seq);
if (!(m_seq & 1))
rcu_read_unlock();
if (need_seqretry(&mount_lock, m_seq)) {
m_seq = 1;
goto restart_mnt;
}
done_seqretry(&mount_lock, m_seq);
if (unlikely(error == 3))
b = *p;
if (b.len == p->len)
prepend_char(&b, '/');
*p = b;
return error;
}
/**
* __d_path - return the path of a dentry
* @path: the dentry/vfsmount to report
* @root: root vfsmnt/dentry
* @buf: buffer to return value in
* @buflen: buffer length
*
* Convert a dentry into an ASCII path name.
*
* Returns a pointer into the buffer or an error code if the
* path was too long.
*
* "buflen" should be positive.
*
* If the path is not reachable from the supplied root, return %NULL.
*/
char *__d_path(const struct path *path,
const struct path *root,
char *buf, int buflen)
{
DECLARE_BUFFER(b, buf, buflen);
prepend_char(&b, 0);
if (unlikely(prepend_path(path, root, &b) > 0))
return NULL;
return extract_string(&b);
}
char *d_absolute_path(const struct path *path,
char *buf, int buflen)
{
struct path root = {};
DECLARE_BUFFER(b, buf, buflen);
prepend_char(&b, 0);
if (unlikely(prepend_path(path, &root, &b) > 1))
return ERR_PTR(-EINVAL);
return extract_string(&b);
}
static void get_fs_root_rcu(struct fs_struct *fs, struct path *root)
{
unsigned seq;
do {
seq = read_seqcount_begin(&fs->seq);
*root = fs->root;
} while (read_seqcount_retry(&fs->seq, seq));
}
/**
* d_path - return the path of a dentry
* @path: path to report
* @buf: buffer to return value in
* @buflen: buffer length
*
* Convert a dentry into an ASCII path name. If the entry has been deleted
* the string " (deleted)" is appended. Note that this is ambiguous.
*
* Returns a pointer into the buffer or an error code if the path was
* too long. Note: Callers should use the returned pointer, not the passed
* in buffer, to use the name! The implementation often starts at an offset
* into the buffer, and may leave 0 bytes at the start.
*
* "buflen" should be positive.
*/
char *d_path(const struct path *path, char *buf, int buflen)
{
DECLARE_BUFFER(b, buf, buflen);
struct path root;
/*
* We have various synthetic filesystems that never get mounted. On
* these filesystems dentries are never used for lookup purposes, and
* thus don't need to be hashed. They also don't need a name until a
* user wants to identify the object in /proc/pid/fd/. The little hack
* below allows us to generate a name for these objects on demand:
*
* Some pseudo inodes are mountable. When they are mounted
* path->dentry == path->mnt->mnt_root. In that case don't call d_dname
* and instead have d_path return the mounted path.
*/
if (path->dentry->d_op && path->dentry->d_op->d_dname &&
(!IS_ROOT(path->dentry) || path->dentry != path->mnt->mnt_root))
return path->dentry->d_op->d_dname(path->dentry, buf, buflen);
rcu_read_lock();
get_fs_root_rcu(current->fs, &root);
if (unlikely(d_unlinked(path->dentry)))
prepend(&b, " (deleted)", 11);
else
prepend_char(&b, 0);
prepend_path(path, &root, &b);
rcu_read_unlock();
return extract_string(&b);
}
EXPORT_SYMBOL(d_path);
/*
* Helper function for dentry_operations.d_dname() members
*/
char *dynamic_dname(struct dentry *dentry, char *buffer, int buflen,
const char *fmt, ...)
{
va_list args;
char temp[64];
int sz;
va_start(args, fmt);
sz = vsnprintf(temp, sizeof(temp), fmt, args) + 1;
va_end(args);
if (sz > sizeof(temp) || sz > buflen)
return ERR_PTR(-ENAMETOOLONG);
buffer += buflen - sz;
return memcpy(buffer, temp, sz);
}
char *simple_dname(struct dentry *dentry, char *buffer, int buflen)
{
DECLARE_BUFFER(b, buffer, buflen);
/* these dentries are never renamed, so d_lock is not needed */
prepend(&b, " (deleted)", 11);
prepend(&b, dentry->d_name.name, dentry->d_name.len);
prepend_char(&b, '/');
return extract_string(&b);
}
/*
* Write full pathname from the root of the filesystem into the buffer.
*/
static char *__dentry_path(const struct dentry *d, struct prepend_buffer *p)
{
const struct dentry *dentry;
struct prepend_buffer b;
int seq = 0;
rcu_read_lock();
restart:
dentry = d;
b = *p;
read_seqbegin_or_lock(&rename_lock, &seq);
while (!IS_ROOT(dentry)) {
const struct dentry *parent = dentry->d_parent;
prefetch(parent);
if (!prepend_name(&b, &dentry->d_name))
break;
dentry = parent;
}
if (!(seq & 1))
rcu_read_unlock();
if (need_seqretry(&rename_lock, seq)) {
seq = 1;
goto restart;
}
done_seqretry(&rename_lock, seq);
if (b.len == p->len)
prepend_char(&b, '/');
return extract_string(&b);
}
char *dentry_path_raw(const struct dentry *dentry, char *buf, int buflen)
{
DECLARE_BUFFER(b, buf, buflen);
prepend_char(&b, 0);
return __dentry_path(dentry, &b);
}
EXPORT_SYMBOL(dentry_path_raw);
char *dentry_path(const struct dentry *dentry, char *buf, int buflen)
{
DECLARE_BUFFER(b, buf, buflen);
if (unlikely(d_unlinked(dentry)))
prepend(&b, "//deleted", 10);
else
prepend_char(&b, 0);
return __dentry_path(dentry, &b);
}
static void get_fs_root_and_pwd_rcu(struct fs_struct *fs, struct path *root,
struct path *pwd)
{
unsigned seq;
do {
seq = read_seqcount_begin(&fs->seq);
*root = fs->root;
*pwd = fs->pwd;
} while (read_seqcount_retry(&fs->seq, seq));
}
/*
* NOTE! The user-level library version returns a
* character pointer. The kernel system call just
* returns the length of the buffer filled (which
* includes the ending '\0' character), or a negative
* error value. So libc would do something like
*
* char *getcwd(char * buf, size_t size)
* {
* int retval;
*
* retval = sys_getcwd(buf, size);
* if (retval >= 0)
* return buf;
* errno = -retval;
* return NULL;
* }
*/
SYSCALL_DEFINE2(getcwd, char __user *, buf, unsigned long, size)
{
int error;
struct path pwd, root;
char *page = __getname();
if (!page)
return -ENOMEM;
rcu_read_lock();
get_fs_root_and_pwd_rcu(current->fs, &root, &pwd);
if (unlikely(d_unlinked(pwd.dentry))) {
rcu_read_unlock();
error = -ENOENT;
} else {
unsigned len;
DECLARE_BUFFER(b, page, PATH_MAX);
prepend_char(&b, 0);
if (unlikely(prepend_path(&pwd, &root, &b) > 0))
prepend(&b, "(unreachable)", 13);
rcu_read_unlock();
len = PATH_MAX - b.len;
if (unlikely(len > PATH_MAX))
error = -ENAMETOOLONG;
else if (unlikely(len > size))
error = -ERANGE;
else if (copy_to_user(buf, b.buf, len))
error = -EFAULT;
else
error = len;
}
__putname(page);
return error;
}
![swh spinner](/static/img/swh-spinner.gif)
Computing file changes ...