linux-kernel - Re: [BUG] ext4 trace events cause NULL pointer dereferences

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: <1279721766.4818.39.camel@gandalf.stny.rr.com>
Date:	Wed, 21 Jul 2010 10:16:06 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
Cc:	Li Zefan <lizf@...fujitsu.com>, Theodore Ts'o <tytso@....edu>,
	LKML <linux-kernel@...r.kernel.org>, linux-ext4@...r.kernel.org,
	Frederic Weisbecker <fweisbec@...il.com>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Subject: Re: [BUG] ext4 trace events cause NULL pointer dereferences

On Wed, 2010-07-21 at 22:31 +0900, KOSAKI Motohiro wrote:
> Hi Steven,

>         if (ac)
> 	        trace_ext4_mb_release_group_pa(ac, pa);
> 
> But, I don't think this is proper fix because we don't want any overhead
> if the tracepoint is disabled.
> 
> So, How do we check NULL in TP_fast_assign()?

You could do:

	TP_fast_assign(
		if (ac) {
	                __entry->dev            = ac->ac_sb->s_dev;
        	        __entry->ino            = ac->ac_inode->i_ino;
        	        __entry->pa_pstart      = pa->pa_pstart;
        	        __entry->pa_len         = pa->pa_len;
		}
        ),

But this just makes the __entry null and wastes the ring buffer.

I may be able to add a __discard_entry that may help. Then we could do
something like this:

	if (ac) {
                __entry->dev            = ac->ac_sb->s_dev;
                __entry->ino            = ac->ac_inode->i_ino;
                __entry->pa_pstart      = pa->pa_pstart;
                __entry->pa_len         = pa->pa_len;
	} else
		__discard_entry;

Does this seem reasonable?

But for now, the wasting the entry seems to be the only choice we have,
or to do as you suggested and have the "if (ac) trace_...", but I don't
like that.

-- Steve



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/