All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: David Laight <David.Laight@ACULAB.COM>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"kernel-janitors@vger.kernel.org"
	<kernel-janitors@vger.kernel.org>
Subject: Re: [patch] wan/x25_asy: integer overflow in x25_asy_change_mtu()
Date: Thu, 17 Jul 2014 11:58:19 +0300	[thread overview]
Message-ID: <20140717085818.GG18338@mwanda> (raw)
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1727688E@AcuExch.aculab.com>

On Thu, Jul 17, 2014 at 08:45:58AM +0000, David Laight wrote:
> From: Dan Carpenter
> > If "newmtu * 2 + 4" is too large then it can cause an integer overflow
> > leading to memory corruption.  Btw, "newmtu" is not allowed to be a
> > negative number because of the check in dev_set_mtu(), so that's ok.
> 
> This still allows large numbers to be used to allocate almost all of
> kernel memory - causing massive issues elsewhere.
> 
> I'd have thought a 'sanity' limit on the mtu would be more appropriate.
> I've no idea which mtu is being changed here, and I can't even remember
> the x.25 protocol well enough if it is an x.25 level 3 limit.
> But I suspect that a 'sanity' bound to 1MB won't cause any grief.
> 

I agree that a sanity check is probably better but I don't think kmalloc
can allocate more than 128k (or something.  It's arch dependent as
well).  So using 1MB is almost no different from my original patch.

What's a better, smaller limit?

regards,
dan carpenter

WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: David Laight <David.Laight@ACULAB.COM>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"kernel-janitors@vger.kernel.org"
	<kernel-janitors@vger.kernel.org>
Subject: Re: [patch] wan/x25_asy: integer overflow in x25_asy_change_mtu()
Date: Thu, 17 Jul 2014 08:58:19 +0000	[thread overview]
Message-ID: <20140717085818.GG18338@mwanda> (raw)
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1727688E@AcuExch.aculab.com>

On Thu, Jul 17, 2014 at 08:45:58AM +0000, David Laight wrote:
> From: Dan Carpenter
> > If "newmtu * 2 + 4" is too large then it can cause an integer overflow
> > leading to memory corruption.  Btw, "newmtu" is not allowed to be a
> > negative number because of the check in dev_set_mtu(), so that's ok.
> 
> This still allows large numbers to be used to allocate almost all of
> kernel memory - causing massive issues elsewhere.
> 
> I'd have thought a 'sanity' limit on the mtu would be more appropriate.
> I've no idea which mtu is being changed here, and I can't even remember
> the x.25 protocol well enough if it is an x.25 level 3 limit.
> But I suspect that a 'sanity' bound to 1MB won't cause any grief.
> 

I agree that a sanity check is probably better but I don't think kmalloc
can allocate more than 128k (or something.  It's arch dependent as
well).  So using 1MB is almost no different from my original patch.

What's a better, smaller limit?

regards,
dan carpenter



  reply	other threads:[~2014-07-17  8:58 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-17  8:03 [patch] wan/x25_asy: integer overflow in x25_asy_change_mtu() Dan Carpenter
2014-07-17  8:03 ` Dan Carpenter
2014-07-17  8:45 ` David Laight
2014-07-17  8:58   ` Dan Carpenter [this message]
2014-07-17  8:58     ` Dan Carpenter
2014-07-17  9:14     ` Eric Dumazet
2014-07-17  9:14       ` Eric Dumazet
2014-07-17 10:50       ` [patch v2] " Dan Carpenter
2014-07-17 10:50         ` Dan Carpenter
2014-07-17 23:48         ` David Miller
2014-07-17 23:48           ` David Miller

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=20140717085818.GG18338@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=netdev@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 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.