All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: tz.stoyanov@gmail.com
Cc: Steven Rostedt <rostedt@goodmis.org>, kernel-janitors@vger.kernel.org
Subject: [bug report] tracing: Add a probe that attaches to trace events
Date: Tue, 24 Aug 2021 14:42:38 +0300	[thread overview]
Message-ID: <20210824114238.GG31143@kili> (raw)

Hello Tzvetomir Stoyanov (VMware),

The patch 7491e2c44278: "tracing: Add a probe that attaches to trace
events" from Aug 19, 2021, leads to the following
Smatch static checker warning:

	kernel/trace/trace_eprobe.c:664 enable_trace_eprobe()
	error: we previously assumed 'file' could be null (see line 652)

kernel/trace/trace_eprobe.c
    638 static int enable_trace_eprobe(struct trace_event_call *call,
    639 			       struct trace_event_file *file)
    640 {
    641 	struct trace_probe *pos, *tp;
    642 	struct trace_eprobe *ep;
    643 	bool enabled;
    644 	int ret = 0;
    645 
    646 	tp = trace_probe_primary_from_call(call);
    647 	if (WARN_ON_ONCE(!tp))
    648 		return -ENODEV;
    649 	enabled = trace_probe_is_enabled(tp);
    650 
    651 	/* This also changes "enabled" state */

Since adding a file sets it to enabled, then shouldn't we set
enabled = true on this path?

    652 	if (file) {
                    ^^^^
Check for NULL

    653 		ret = trace_probe_add_file(tp, file);
    654 		if (ret)
    655 			return ret;
    656 	} else
    657 		trace_probe_set_flag(tp, TP_FLAG_PROFILE);
    658 
    659 	if (enabled)
    660 		return 0;
    661 
    662 	list_for_each_entry(pos, trace_probe_probe_list(tp), list) {
    663 		ep = container_of(pos, struct trace_eprobe, tp);
--> 664 		ret = enable_eprobe(ep, file);
                                                ^^^^
Unchecked dereference.  See also:

kernel/trace/trace_eprobe.c:705 disable_trace_eprobe() error: we previously assumed 'file' could be null (see line 693)

    665 		if (ret)
    666 			break;
    667 		enabled = true;
    668 	}
    669 
    670 	if (ret) {
    671 		/* Failed to enable one of them. Roll back all */
    672 		if (enabled)
    673 			disable_eprobe(ep, file->tr);
    674 		if (file)
    675 			trace_probe_remove_file(tp, file);
    676 		else
    677 			trace_probe_clear_flag(tp, TP_FLAG_PROFILE);
    678 	}
    679 
    680 	return ret;
    681 }

regards,
dan carpenter

             reply	other threads:[~2021-08-24 11:43 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24 11:42 Dan Carpenter [this message]
  -- strict thread matches above, loose matches on Subject: below --
2021-08-24  9:07 [bug report] tracing: Add a probe that attaches to trace events Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210824114238.GG31143@kili \
    --to=dan.carpenter@oracle.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tz.stoyanov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.