From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934939AbcCJDei (ORCPT ); Wed, 9 Mar 2016 22:34:38 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:42288 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754411AbcCJDeg (ORCPT ); Wed, 9 Mar 2016 22:34:36 -0500 Date: Wed, 9 Mar 2016 19:34:35 -0800 From: Greg KH To: Jiang Lu Cc: linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org Subject: Re: [v2] serial_core:recognize invalid pointer from userspace Message-ID: <20160310033435.GA28983@kroah.com> References: <1457579843-29676-1-git-send-email-lu.jiang@windriver.com> <1457579843-29676-2-git-send-email-lu.jiang@windriver.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1457579843-29676-2-git-send-email-lu.jiang@windriver.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 10, 2016 at 11:17:23AM +0800, Jiang Lu wrote: > compat_ioctl use 0xffffffff as a magic number to mark invalid pointer > for iomem_base in serial_struct when truncating a 64bit pointer into > 32bit. > > Serial driver need recognize this invalid pointer when parsing > serial_struct from userspace. > > Signed-off-by: Jiang Lu > --- > drivers/tty/serial/serial_core.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c > index a5d545e..d293536 100644 > --- a/drivers/tty/serial/serial_core.c > +++ b/drivers/tty/serial/serial_core.c > @@ -745,6 +745,9 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port, > * allocations, we should treat type changes the same as > * IO port changes. > */ > + if ((unsigned long)new_info->iomem_base == 0xffffffff) > + new_info->iomem_base = (void *)(unsigned long)uport->mapbase; This looks really odd to me, why do we care about userspace issues here? Shouldn't the compat ioctl code have handled this already all for us? And why set it to mapbase? Just to keep it from being changed? this worries me... greg k-h