Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753268AbdLDKeV (ORCPT ); Mon, 4 Dec 2017 05:34:21 -0500 Received: from aserp2130.oracle.com ([141.146.126.79]:60060 "EHLO aserp2130.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751718AbdLDKeR (ORCPT ); Mon, 4 Dec 2017 05:34:17 -0500 Date: Mon, 4 Dec 2017 13:33:56 +0300 From: Dan Carpenter To: Simon =?iso-8859-1?Q?Sandstr=F6m?= Cc: gregkh@linuxfoundation.org, devel@driverdev.osuosl.org, linux@Wolf-Entwicklungen.de, linux-kernel@vger.kernel.org Subject: Re: [PATCH 6/6] staging: pi433: Rename enum modShaping in rf69_enum.h Message-ID: <20171204103356.cxmblmuvva3kjst6@mwanda> References: <20171203151726.16639-1-simon@nikanor.nu> <20171203151726.16639-7-simon@nikanor.nu> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20171203151726.16639-7-simon@nikanor.nu> User-Agent: NeoMutt/20170609 (1.8.3) X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=8734 signatures=668637 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1712040152 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1814 Lines: 48 On Sun, Dec 03, 2017 at 04:17:26PM +0100, Simon Sandstr?m wrote: > diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h > index 34ff0d4807bd..bcfe29840889 100644 > --- a/drivers/staging/pi433/pi433_if.h > +++ b/drivers/staging/pi433/pi433_if.h > @@ -63,7 +63,7 @@ struct pi433_tx_cfg { > __u16 bit_rate; > __u32 dev_frequency; > enum modulation modulation; > - enum modShaping modShaping; > + enum mod_shaping mod_shaping; > I looked at how mod_shaping is set and the only place is in the ioctl: 789 case PI433_IOC_WR_TX_CFG: 790 if (copy_from_user(&instance->tx_cfg, argp, 791 sizeof(struct pi433_tx_cfg))) 792 return -EFAULT; 793 break; We just write over the whole config. Including important things like rx_cfg.fixed_message_length. There is no locking so when we do things like: 385 /* fixed or unlimited length? */ 386 if (dev->rx_cfg.fixed_message_length != 0) 387 { 388 if (dev->rx_cfg.fixed_message_length > dev->rx_buffer_size) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ check 389 { 390 retval = -1; 391 goto abort; 392 } 393 bytes_total = dev->rx_cfg.fixed_message_length; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ set this in the ioctl after the check but before this line and it looks like a security problem. 394 dev_dbg(dev->dev,"rx: msg len set to %d by fixed length", bytes_total); 395 } Anyway, I guess this patch is fine. regards, dan carpenter