netdev - Re: [patch] wan/x25_asy: integer overflow in x25_asy_change

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: <20140717085818.GG18338@mwanda>
Date:	Thu, 17 Jul 2014 11:58:19 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	David Laight <David.Laight@...LAB.COM>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>
Subject: Re: [patch] wan/x25_asy: integer overflow in x25_asy_change_mtu()

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


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html