Distributed Replicated Block Device (DRBD) development
 help / color / mirror / Atom feed
* [Drbd-dev] [bug report] drbd: Backport the "events2" command
@ 2017-02-23 15:55 Dan Carpenter
  2017-02-24 15:29 ` Lars Ellenberg
  0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2017-02-23 15:55 UTC (permalink / raw)
  To: agruen; +Cc: drbd-dev

Hello Andreas Gruenbacher,

The patch a29728463b25: "drbd: Backport the "events2" command" from
Jul 31, 2014, leads to the following static checker warning:

	drivers/block/drbd/drbd_nl.c:4934 get_initial_state()
	error: dereferencing freed memory 'skb'

drivers/block/drbd/drbd_nl.c
  4880  static int get_initial_state(struct sk_buff *skb, struct netlink_callback *cb)
  4881  {
  4882          struct drbd_state_change *state_change = (struct drbd_state_change *)cb->args[0];
  4883          unsigned int seq = cb->args[2];
  4884          unsigned int n;
  4885          enum drbd_notification_type flags = 0;
  4886  
  4887          /* There is no need for taking notification_mutex here: it doesn't
  4888             matter if the initial state events mix with later state chage
  4889             events; we can always tell the events apart by the NOTIFY_EXISTS
  4890             flag. */
  4891  
  4892          cb->args[5]--;
  4893          if (cb->args[5] == 1) {
  4894                  notify_initial_state_done(skb, seq);
                                                  ^^^
skb is freed on error inside notify_initial_state_done().

  4895                  goto out;
  4896          }
  4897          n = cb->args[4]++;
  4898          if (cb->args[4] < cb->args[3])
  4899                  flags |= NOTIFY_CONTINUES;
  4900          if (n < 1) {
  4901                  notify_resource_state_change(skb, seq, state_change->resource,
  4902                                               NOTIFY_EXISTS | flags);
  4903                  goto next;
  4904          }
  4905          n--;
  4906          if (n < state_change->n_connections) {
  4907                  notify_connection_state_change(skb, seq, &state_change->connections[n],
  4908                                                 NOTIFY_EXISTS | flags);
  4909                  goto next;
  4910          }
  4911          n -= state_change->n_connections;
  4912          if (n < state_change->n_devices) {
  4913                  notify_device_state_change(skb, seq, &state_change->devices[n],
  4914                                             NOTIFY_EXISTS | flags);
  4915                  goto next;
  4916          }
  4917          n -= state_change->n_devices;
  4918          if (n < state_change->n_devices * state_change->n_connections) {
  4919                  notify_peer_device_state_change(skb, seq, &state_change->peer_devices[n],
  4920                                                  NOTIFY_EXISTS | flags);
  4921                  goto next;
  4922          }
  4923  
  4924  next:
  4925          if (cb->args[4] == cb->args[3]) {
  4926                  struct drbd_state_change *next_state_change =
  4927                          list_entry(state_change->list.next,
  4928                                     struct drbd_state_change, list);
  4929                  cb->args[0] = (long)next_state_change;
  4930                  cb->args[3] = notifications_for_state_change(next_state_change);
  4931                  cb->args[4] = 0;
  4932          }
  4933  out:
  4934          return skb->len;
                       ^^^^^^^^
Dereference.

  4935  }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [Drbd-dev] [bug report] drbd: Backport the "events2" command
@ 2017-03-06 15:22 Dan Carpenter
  2017-03-06 15:58 ` Lars Ellenberg
  0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2017-03-06 15:22 UTC (permalink / raw)
  To: agruen; +Cc: drbd-dev

Hello Andreas Gruenbacher,

The patch a29728463b25: "drbd: Backport the "events2" command" from
Jul 31, 2014, leads to the following static checker warning:

	drivers/block/drbd/drbd_nl.c:4934 get_initial_state()
	error: dereferencing freed memory 'skb'

drivers/block/drbd/drbd_nl.c
  4841  static void notify_initial_state_done(struct sk_buff *skb, unsigned int seq)
  4842  {
  4843          struct drbd_genlmsghdr *dh;
  4844          int err;
  4845  
  4846          err = -EMSGSIZE;
  4847          dh = genlmsg_put(skb, 0, seq, &drbd_genl_family, 0, DRBD_INITIAL_STATE_DONE);
  4848          if (!dh)
  4849                  goto nla_put_failure;
  4850          dh->minor = -1U;
  4851          dh->ret_code = NO_ERROR;
  4852          if (nla_put_notification_header(skb, NOTIFY_EXISTS))
  4853                  goto nla_put_failure;
  4854          genlmsg_end(skb, dh);
  4855          return;
  4856  
  4857  nla_put_failure:
  4858          nlmsg_free(skb);

We free this on error, but it's a void function so it seems like the
callers just assume it succeeded leading to a use after free bug.

(It's also possible that I have misunderstood the refcounting here).

  4859          pr_err("Error %d sending event. Event seq:%u\n", err, seq);
  4860  }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-06 15:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-23 15:55 [Drbd-dev] [bug report] drbd: Backport the "events2" command Dan Carpenter
2017-02-24 15:29 ` Lars Ellenberg
  -- strict thread matches above, loose matches on Subject: below --
2017-03-06 15:22 Dan Carpenter
2017-03-06 15:58 ` Lars Ellenberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox