From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933506AbcCKCbS (ORCPT ); Thu, 10 Mar 2016 21:31:18 -0500 Received: from mail5.windriver.com ([192.103.53.11]:41868 "EHLO mail5.wrs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933259AbcCKCbP (ORCPT ); Thu, 10 Mar 2016 21:31:15 -0500 Reply-To: Subject: Re: [v2] serial_core:recognize invalid pointer from userspace References: <1457579843-29676-1-git-send-email-lu.jiang@windriver.com> <1457579843-29676-2-git-send-email-lu.jiang@windriver.com> <20160310033435.GA28983@kroah.com> <56E0FE77.1020303@windriver.com> <20160310134647.7053a520@lxorguk.ukuu.org.uk> To: One Thousand Gnomes CC: Greg KH , , From: "Lu.Jiang" Message-ID: <56E22DE1.80101@windriver.com> Date: Fri, 11 Mar 2016 10:30:57 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20160310134647.7053a520@lxorguk.ukuu.org.uk> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [128.224.163.23] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016年03月10日 21:46, One Thousand Gnomes wrote: >> When userspace get setting with TIOCGSERIAL, >> >> 1.On 64bit kernel + 32bit rootfs, compat ioctl code use 0xffffffff to >> mark invalid conversion. > Start at the beginning. What is being passed back and forth that causes > the problem. What memory address does your uart have ? In fs/compat_ioctl.c, serial_struct_ioctl() use following code for convert a 64bit pointer into 32bit userspace pointer if (cmd == TIOCGSERIAL && err >= 0) { if (copy_in_user(ss32, ss, offsetof(SS32, iomem_base)) || get_user(iomem_base, &ss->iomem_base)) return -EFAULT; base = (unsigned long)iomem_base >> 32 ? 0xffffffff : (unsigned)(unsigned long)iomem_base; If iomem_base exceed 4G, TIOCGSERIAL didn't return right value for iomem_base. it use following code to covert 32bit to 64bit. if (cmd == TIOCSSERIAL) { if (copy_in_user(ss, ss32, offsetof(SS32, iomem_base)) || get_user(udata, &ss32->iomem_base)) return -EFAULT; iomem_base = compat_ptr(udata); static inline void __user *compat_ptr(compat_uptr_t uptr) { return (void __user *)(unsigned long)uptr; } You can see userspace application didn't get valid iomem_base via TIOCGSERAIL. When issuing TIOCSSERIAL, application can not provide a workable value for kernel. It means TIOCSSERIAL can not work. Typical serial setting application setserial is depend on those 2 ioctls. > >> 2.On 32bit kernel with 64bit physical address, uart_get_info() in >> serial_core will truncate a 64bit address into 32bit pointer with >> following code: >> retinfo->iomem_base = (void *)(unsigned long)uport->mapbase; >> >> Then when setting with TIOCSSERIAL ioctl, application can not provide >> original value for iomem_base, it leads setserial can not work on such >> boards. > Yes - it's a legacy feature that isn't really needed on 64bit systems. > >> I don't know why kernel should expose this value to userspace, and seems >> no need for userspace application to update an physical address for driver. >> Can we consider drop this member in handle for TIOCSSERIAL ioctl. Please >> see a rough patch in attachment. > There are old PC and embedded cases from the time before devicetree and > ACPI were the default. It's now an API people rely upon. > > It would make sense I think to return 0 for the base address if it > exceeds 32bits, and also to ignore a TIOCSSERIAL that passes 0 as the > address back. Would that fix your use case ? If boards relay on this TIOCSSERIAL only working on 32bit, this solution is workable. Thanks Jiang Lu > > Alan >