Re: [patch 014/173] mm, tracing: record slab name for kmem_cache_free()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 25 Feb 2021 15:07:50 +0800
Jacob Wen <jian.w.wen@xxxxxxxxxx> wrote:

> > I wonder if we can add something to checkpatch that can check if
> > TP_printk() has a call to "%s" where it references a __entry->xxx and
> > not a __get_str(), and will warn about it.  
> That's helpful for me who don't know "%s" of TP_printk is special.

Here's nothing special about %s in TP_printk. It uses the same code as
printk() and what other string formatters use.

What is special is that the print is on data that is stored from a previous
time.

TP_fast_assign() / TP_printk() is basically this:

struct entry {
	char *name;
} entry;


TP_fast_assign() {
	entry.name = slab->name;
}

TP_printk() {
	printk("%s", entry.name);
}


Where TP_printk() can be called some time in the future when a user asks
for it. If the slab->name is freed, then the entry.name will be pointing to
stale data, and you don't want to call printk() on that!

Thus, the "%s" in TP_printk() is nothing special, it's the fact that the
data it reads is called much later in time from when that data was
recorded. Which means, you can not rely on any dereferencing of pointers.

The __string() __assign_str() and __get_str() macros are helpers to easily
store strings in the ring buffer, as that is a common practice in the
trace events.

-- Steve


> >
> > There a a few cases where its OK. Like RCU uses a TPS() macro around
> > strings it passes into the tracepoint, which is used for strings that
> > never are freed, and maps the string pointer to the string for user
> > space. But RCU is the only user of that I believe.
> >



[Index of Archives]     [Kernel Archive]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]

  Powered by Linux