Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753566AbaD1Llw (ORCPT ); Mon, 28 Apr 2014 07:41:52 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:28589 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751791AbaD1Llu (ORCPT ); Mon, 28 Apr 2014 07:41:50 -0400 Date: Mon, 28 Apr 2014 14:41:26 +0300 From: Dan Carpenter To: walter harms Cc: Greg Kroah-Hartman , Peng Tao , Andreas Dilger , Oleg Drokin , "John L. Hammond" , Dmitry Eremin , Jinshan Xiong , Dulshani Gunawardhana , devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [patch 2/2 v2] staging: lustre: integer overflow in obd_ioctl_is_invalid() Message-ID: <20140428114126.GK4963@mwanda> References: <20140428105858.GA31925@mwanda> <535E3CF7.9030609@bfs.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <535E3CF7.9030609@bfs.de> User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: ucsinet21.oracle.com [156.151.31.93] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Apr 28, 2014 at 01:35:19PM +0200, walter harms wrote: > > > Am 28.04.2014 12:58, schrieb Dan Carpenter: > > The obd_ioctl_getdata() function caps "data->ioc_len" at > > OBD_MAX_IOCTL_BUFFER and then calls this obd_ioctl_is_invalid() to check > > that the other values inside data are valid. > > > > There are several lengths inside data but when they are added together > > they must not be larger than "data->ioc_len". The checks against > > "(data->ioc_inllen1 > (1<<30))" are supposed to ensure that the addition > > does not have an integer overflow. But "(1<<30) * 4" actually can > > overflow 32 bits, so the checks are insufficient. > > > > I have changed it to "> OBD_MAX_IOCTL_BUFFER" instead. > > > > Signed-off-by: Dan Carpenter > > --- > > v2: Updated the error messages as Walter Harms pointed out. > > > > diff --git a/drivers/staging/lustre/lustre/include/lustre_lib.h b/drivers/staging/lustre/lustre/include/lustre_lib.h > > index bdc9812..3c26bbd 100644 > > --- a/drivers/staging/lustre/lustre/include/lustre_lib.h > > +++ b/drivers/staging/lustre/lustre/include/lustre_lib.h > > @@ -179,24 +179,25 @@ static inline int obd_ioctl_packlen(struct obd_ioctl_data *data) > > > > static inline int obd_ioctl_is_invalid(struct obd_ioctl_data *data) > > { > > - if (data->ioc_len > (1<<30)) { > > - CERROR("OBD ioctl: ioc_len larger than 1<<30\n"); > > + if (data->ioc_len > OBD_MAX_IOCTL_BUFFER) { > > + CERROR("OBD ioctl: ioc_len larger than %d\n", > > + OBD_MAX_IOCTL_BUFFER); > > return 1; > > } > > - if (data->ioc_inllen1 > (1<<30)) { > > - CERROR("OBD ioctl: ioc_inllen1 larger than 1<<30\n"); > > + if (data->ioc_inllen1 > OBD_MAX_IOCTL_BUFFER) { > > + CERROR("OBD ioctl: ioc_inllen1 larger than ioc_len\n"); > > return 1; > > } > > The error mention ioc_len the compare is OBD_MAX_IOCTL_BUFFER ? > Is that intentional ? Yep. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/