linux-kernel - Re: [PATCH 1/2] [BUGFIX] perf probe: Fix --list option to show events only with uprobe events

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: <20140813144801.GA2718@kernel.org>
Date:	Wed, 13 Aug 2014 11:48:01 -0300
From:	Arnaldo Carvalho de Melo <acme@...nel.org>
To:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Cc:	Namhyung Kim <namhyung@...nel.org>, Naohiro Aota <naota@...sp.net>,
	Ingo Molnar <mingo@...hat.com>,
	Paul Mackerras <paulus@...ba.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] [BUGFIX] perf probe: Fix --list option to show
 events only with uprobe events

Em Wed, Aug 13, 2014 at 11:21:34PM +0900, Masami Hiramatsu escreveu:
> (2014/08/13 14:22), Namhyung Kim wrote:
> > On Wed, 13 Aug 2014 00:50:55 +0000, Masami Hiramatsu wrote:
> >> +	if (kp_fd < 0 && up_fd < 0) {
> >> +		/* Both kprobes and uprobes are disabled, warn it. */
> >> +		if (kp_fd == -ENOTSUP && up_fd == -ENOTSUP)
> >> +			pr_warning("Debugfs is not mounted.\n");
> >> +		else if (kp_fd == -ENOENT && up_fd == -ENOENT)
> >> +			pr_warning("Please rebuild kernel with "
> >> +				   "CONFIG_KPROBE_EVENTS or/and "
> >> +				   "CONFIG_UPROBE_EVENTS.\n");
> >> +		else
> >> +			pr_warning("Failed to open kprobe events: %s.\n" \
> >> +				   "Failed to open uprobe events: %s.\n",
> >> +				   strerror(-kp_fd), strerror(-up_fd));
> > 
> > It seems the second strerror() might overwrite the message of the
> > first.  You'd better using strerror_r() IMHO.

Well spotted!
 
> Oops, right, it must use the same buffer...
> But instead of using strerror_r, we can call pr_warning twice. Or should we
> better replace all strerror to strerror_r in perf? (it should be another series)

Well, don't introduce new strerror() uses, we have threads in perf
already and if both try to use strerror() for different reasons, say the
UI to print something to the user and some logging/debugging thread do
it to the disk, we may race.

So, in this case, please use strerror_r() and if you feel like
contributing the changes to any other place where strerror() is still
used, you are welcome to do so at a later patch :)

- Arnaldo
--
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/