public inbox for kernel-janitors@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: kernel-janitors@vger.kernel.org
Subject: [report] buffer overflow in capmode.c rx() function
Date: Mon, 08 Feb 2016 21:08:11 +0000	[thread overview]
Message-ID: <20160208210811.GA30822@mwanda> (raw)

Here is the static checker warning.

	drivers/net/arcnet/capmode.c:78 rx()
	error: __memcpy() 'pktbuf + 4 +  + 4' too small (11 vs 15)

drivers/net/arcnet/capmode.c
    42  static void rx(struct net_device *dev, int bufnum,
    43                 struct archdr *pkthdr, int length)
    44  {
    45          struct arcnet_local *lp = netdev_priv(dev);
    46          struct sk_buff *skb;
    47          struct archdr *pkt = pkthdr;
                               ^^^^^^^^^^^^
Never used assignment.

    48          char *pktbuf, *pkthdrbuf;
    49          int ofs;
    50  
    51          arc_printk(D_DURING, dev, "it's a raw(cap) packet (length=%d)\n",
    52                     length);
    53  
    54          if (length >= MinTU)
    55                  ofs = 512 - length;
    56          else
    57                  ofs = 256 - length;
    58  
    59          skb = alloc_skb(length + ARC_HDR_SIZE + sizeof(int), GFP_ATOMIC);
    60          if (!skb) {
    61                  dev->stats.rx_dropped++;
    62                  return;
    63          }
    64          skb_put(skb, length + ARC_HDR_SIZE + sizeof(int));
    65          skb->dev = dev;
    66          skb_reset_mac_header(skb);
    67          pkt = (struct archdr *)skb_mac_header(skb);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Assigned here.

    68          skb_pull(skb, ARC_HDR_SIZE);
    69  
    70          /* up to sizeof(pkt->soft) has already been copied from the card
    71           * squeeze in an int for the cap encapsulation
    72           * use these variables to be sure we count in bytes, not in
    73           * sizeof(struct archdr)
    74           */
    75          pktbuf = (char *)pkt;
    76          pkthdrbuf = (char *)pkthdr;
    77          memcpy(pktbuf, pkthdrbuf, ARC_HDR_SIZE + sizeof(pkt->soft.cap.proto));
    78          memcpy(pktbuf + ARC_HDR_SIZE + sizeof(pkt->soft.cap.proto) + sizeof(int),
                                                                             ^^^^^^^^^^^

It looks like we break the memcpy into two peices so that we don't write
over this sizeof(int).


    79                 pkthdrbuf + ARC_HDR_SIZE + sizeof(pkt->soft.cap.proto),
    80                 sizeof(struct archdr) - ARC_HDR_SIZE - sizeof(pkt->soft.cap.proto));

But shouldn't the length value also say - sizeof(int)?  It looks like
we end up corrupting stack in the arcnet_rx() function.

    81  
    82          if (length > sizeof(pkt->soft))
    83                  lp->hw.copy_from_card(dev, bufnum, ofs + sizeof(pkt->soft),
    84                                        pkt->soft.raw + sizeof(pkt->soft)
    85                                        + sizeof(int),
    86                                        length - sizeof(pkt->soft));
    87  

regards,
dan carpenter

                 reply	other threads:[~2016-02-08 21:08 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20160208210811.GA30822@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=kernel-janitors@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox