Message ID | 20221222044925.27846-1-quic_vdadhani@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:e747:0:0:0:0:0 with SMTP id c7csp138372wrn; Wed, 21 Dec 2022 21:02:12 -0800 (PST) X-Google-Smtp-Source: AMrXdXs3BuAlqEkeeVaFHXOt8U19qtL45qFJVw0oycQo6FrgKLVkoczXKx6v8ilkPfMbjfhadPKv X-Received: by 2002:a17:90a:cb16:b0:225:b36b:caef with SMTP id z22-20020a17090acb1600b00225b36bcaefmr1160980pjt.23.1671685332079; Wed, 21 Dec 2022 21:02:12 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1671685332; cv=none; d=google.com; s=arc-20160816; b=EHF34TGGE9Q0GRjXdgGLP1cDz6xwuxZrBIRNE7yYkPuRot9DuF7kH8dtAlkfirAvby 52IEcmgwtK5/7xT/6WnBrDsAev7flnIMKEJ4+l8YrjbZwZQilJnhJ4D+7k/P6jtM4WlZ v/4bm5bMRN6EtzyulRQYb3zUEKl84YIqkRp6lxsyar4eino9wtXu+ewpV+MEaXgvO8W2 O+X1Ri8bMQAskvHQHNZ3UroFhtXGC82XS/CuN5KOs1+OWVSQp44AJcxVuYshzp06WY0d MZGveRs5x0LCRu26fkikOtI4JtXgvUkqysImy74BKVRy0+2Yvfj6nNoViRDNw3UYYvIK lJXQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:message-id:date:subject:cc:to:from :dkim-signature; bh=8NKz2ilz1nUXqtBqiyLd1bHJ5FljdNQMK8Gvb39Gb+M=; b=uuT7RGaOkQSQW4C8anum1OXBTmLA//WZpQ5ncMTICvPqwbm3sfVM9vOqmO8v4kgt3F 152rYplJnOS+eMeJou3z2wjijwEalbyejYbPYjuFSE2+xZn9+U31SP40UX37+er9u3T6 DZVm2EvkfoTHoKY3dYe1r0EuG2Do/az5qMWMpVLAfbcGJSBzQm380uF063w41f/TFNI1 jMM4FGVTWQf8f6KJoBA/eo2uXk3KYkU8lxZmnmZCgKncoptNYoikPbC0QzXS3bLZcgc4 +82CtbwmXIq+DEbKakchbZsY0ew5hS+tQaImuGyQgwxtsS0BEx4BYv+PUz7yTlPKHE6J p8ag== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=n+Udh3cQ; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id 187-20020a6300c4000000b00439f026cf5dsi18188024pga.612.2022.12.21.21.01.55; Wed, 21 Dec 2022 21:02:12 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=n+Udh3cQ; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234888AbiLVEts (ORCPT <rfc822;pacteraone@gmail.com> + 99 others); Wed, 21 Dec 2022 23:49:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54834 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234459AbiLVEto (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 21 Dec 2022 23:49:44 -0500 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 458211A80E; Wed, 21 Dec 2022 20:49:44 -0800 (PST) Received: from pps.filterd (m0279863.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 2BM4bgBC032093; Thu, 22 Dec 2022 04:49:41 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : to : cc : subject : date : message-id; s=qcppdkim1; bh=8NKz2ilz1nUXqtBqiyLd1bHJ5FljdNQMK8Gvb39Gb+M=; b=n+Udh3cQtavIsBd7Gvp/Y8/e7hTm+JcmZW9dvs+paUTYlrdHICvLScco69KpADrCZxjT 9YMrKAwIYVadYRAY3zDQZVd6U7FRmxwJX9D056yDlyPBZpvxW+F3UMDqGdV+J57uSj8y VjtYaVqNa8nlvC6HssPwCVZ9MAZv3wR7xZGwaSt8pTZak4XkSz6rJ55W28UA4Z3mZGxd jAIdaZNLgb92z7yvYGOMr7KHQNOC4DESFVukr2f8zvsbbyqgRefifvSw5vQPp2wxh2IX EEvDaWNfKUWtqKLHgyCXDJsSPI/aLSZTGqDaNo3Z1WCWa+JL6xpr36dcCAqEHw2rm23a Rw== Received: from apblrppmta02.qualcomm.com (blr-bdr-fw-01_GlobalNAT_AllZones-Outside.qualcomm.com [103.229.18.19]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3mm5r2h9x4-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 22 Dec 2022 04:49:41 +0000 Received: from pps.filterd (APBLRPPMTA02.qualcomm.com [127.0.0.1]) by APBLRPPMTA02.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTP id 2BM4na0u017965; Thu, 22 Dec 2022 04:49:36 GMT Received: from pps.reinject (localhost [127.0.0.1]) by APBLRPPMTA02.qualcomm.com (PPS) with ESMTPS id 3mh6ukhx2p-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 22 Dec 2022 04:49:36 +0000 Received: from APBLRPPMTA02.qualcomm.com (APBLRPPMTA02.qualcomm.com [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 2BM4naUo017958; Thu, 22 Dec 2022 04:49:36 GMT Received: from hu-maiyas-hyd.qualcomm.com (hu-vdadhani-hyd.qualcomm.com [10.213.106.28]) by APBLRPPMTA02.qualcomm.com (PPS) with ESMTP id 2BM4nauk017957; Thu, 22 Dec 2022 04:49:36 +0000 Received: by hu-maiyas-hyd.qualcomm.com (Postfix, from userid 4047106) id 87BE65001C2; Thu, 22 Dec 2022 10:19:35 +0530 (+0530) From: Viken Dadhaniya <quic_vdadhani@quicinc.com> To: gregkh@linuxfoundation.org, jirislaby@kernel.org, linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org Cc: quic_msavaliy@quicinc.com, quic_vnivarth@quicinc.com, quic_vtanuku@quicinc.com, quic_arandive@quicinc.com, Viken Dadhaniya <quic_vdadhani@quicinc.com> Subject: [PATCH V1 1/1] Serial: core: Add compat ioctl support Date: Thu, 22 Dec 2022 10:19:25 +0530 Message-Id: <20221222044925.27846-1-quic_vdadhani@quicinc.com> X-Mailer: git-send-email 2.17.1 X-QCInternal: smtphost X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: ZhoE1F2HzIAuvtmtE_rav98Md3AnjTWj X-Proofpoint-ORIG-GUID: ZhoE1F2HzIAuvtmtE_rav98Md3AnjTWj X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-12-22_01,2022-12-21_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 lowpriorityscore=0 priorityscore=1501 phishscore=0 mlxlogscore=999 malwarescore=0 bulkscore=0 impostorscore=0 spamscore=0 clxscore=1011 adultscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000 definitions=main-2212220042 X-Spam-Status: No, score=-2.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1752889118773893167?= X-GMAIL-MSGID: =?utf-8?q?1752889118773893167?= |
Series |
[V1,1/1] Serial: core: Add compat ioctl support
|
|
Commit Message
Viken Dadhaniya
Dec. 22, 2022, 4:49 a.m. UTC
Current serial core driver doesn't support compat_ioctl
due to which 32-bit application is not able to send
ioctls to driver on a 64-bit platform.
Added compat_ioctl support in serial core to handle
ioctls from 32-bit applications on a 64-bit platform.
Signed-off-by: Viken Dadhaniya <quic_vdadhani@quicinc.com>
---
drivers/tty/serial/serial_core.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
Comments
On Thu, Dec 22, 2022 at 10:19:25AM +0530, Viken Dadhaniya wrote: > Current serial core driver doesn't support compat_ioctl > due to which 32-bit application is not able to send > ioctls to driver on a 64-bit platform. Are you sure? > Added compat_ioctl support in serial core to handle > ioctls from 32-bit applications on a 64-bit platform. > > Signed-off-by: Viken Dadhaniya <quic_vdadhani@quicinc.com> > --- > drivers/tty/serial/serial_core.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c > index b9fbbee598b8..5ffa0798db3b 100644 > --- a/drivers/tty/serial/serial_core.c > +++ b/drivers/tty/serial/serial_core.c > @@ -1601,6 +1601,22 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg) > return ret; > } > > +/** > + * uart_compat_ioctl: uart compat IOCTL function. > + * @tty: pointer to tty structure. > + * @cmd: command code passed by user-space. > + * @arg: argument passed by user-space. > + * > + * This function will call normal uart IOCTL. > + * > + * Return: 0 for success, Negative number for error condition. Why create kernel doc for a static function? uart_ioctl() does not have this, right? > + */ > +static long > +uart_compat_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg) > +{ > + return (long)uart_ioctl(tty, cmd, arg); > +}; Really? Why has this never shown up before as an issue? How was this tested? What is currently broken that now works properly (or the other way around?) This patch implies that _every_ driver with an ioctl must create a compat_ioctl() callback, are you sure that is the case? thanks, greg k-h
Hi Greg, Please find response inline. > -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Thursday, December 22, 2022 11:19 AM > To: Viken Dadhaniya (QUIC) <quic_vdadhani@quicinc.com> > Cc: jirislaby@kernel.org; linux-serial@vger.kernel.org; linux- > kernel@vger.kernel.org; Mukesh Savaliya (QUIC) <quic_msavaliy@quicinc.com>; > Vijaya Krishna Nivarthi (Temp) (QUIC) <quic_vnivarth@quicinc.com>; Visweswara > Tanuku (QUIC) <quic_vtanuku@quicinc.com>; Aniket RANDIVE (QUIC) > <quic_arandive@quicinc.com> > Subject: Re: [PATCH V1 1/1] Serial: core: Add compat ioctl support > > WARNING: This email originated from outside of Qualcomm. Please be wary of > any links or attachments, and do not enable macros. > > On Thu, Dec 22, 2022 at 10:19:25AM +0530, Viken Dadhaniya wrote: > > Current serial core driver doesn't support compat_ioctl due to which > > 32-bit application is not able to send ioctls to driver on a 64-bit > > platform. > > Are you sure? > > > Added compat_ioctl support in serial core to handle ioctls from 32-bit > > applications on a 64-bit platform. > > > > Signed-off-by: Viken Dadhaniya <quic_vdadhani@quicinc.com> > > --- > > drivers/tty/serial/serial_core.c | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/drivers/tty/serial/serial_core.c > > b/drivers/tty/serial/serial_core.c > > index b9fbbee598b8..5ffa0798db3b 100644 > > --- a/drivers/tty/serial/serial_core.c > > +++ b/drivers/tty/serial/serial_core.c > > @@ -1601,6 +1601,22 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, > unsigned long arg) > > return ret; > > } > > > > +/** > > + * uart_compat_ioctl: uart compat IOCTL function. > > + * @tty: pointer to tty structure. > > + * @cmd: command code passed by user-space. > > + * @arg: argument passed by user-space. > > + * > > + * This function will call normal uart IOCTL. > > + * > > + * Return: 0 for success, Negative number for error condition. > > Why create kernel doc for a static function? uart_ioctl() does not have this, > right? > I will remove documentation in V2 patch. > > + */ > > +static long > > +uart_compat_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned > > +long arg) { > > + return (long)uart_ioctl(tty, cmd, arg); }; > > Really? Why has this never shown up before as an issue? > We have only seen this problem with 32-bit applications running on 64-bit kernels. For the first time, we are combining a 64-bit kernel with a 32-bit user space application. > How was this tested? What is currently broken that now works properly (or the > other way around?) > We are running 32-bit user-space application on 64-bit kernel, and vendor specific ioctl command issued by application is not reaching the vendor driver. Without these changes, the IOCTL call was returning from the tty framework rather than reaching to the vendor driver. IOCTL call is successfully reaching the vendor uart driver via tty framework after adding compat ioctl and functionality is working as expected and below is the dump stack with compat ioctl. [ 1265.554002] Hardware name: Qualcomm Technologies, Inc. Monaco IDP V1.0 (DT) [ 1265.554008] Call trace: [ 1265.554011] dump_backtrace.cfi_jt+0x0/0x8 [ 1265.554023] show_stack+0x1c/0x2c [ 1265.554032] dump_stack_lvl+0x80/0xc8 [ 1265.554041] dump_stack+0x1c/0x2c [ 1265.554049] msm_geni_serial_ioctl+0x2cc/0x3d0 [msm_geni_serial] [ 1265.554099] uart_ioctl+0x1a4/0x1d8 [ 1265.554109] uart_compat_ioctl+0x14/0x28 [ 1265.554117] tty_compat_ioctl+0x1a8/0x2dc [ 1265.554125] __arm64_compat_sys_ioctl+0x158/0x1d0 [ 1265.554132] invoke_syscall+0x60/0x150 [ 1265.554140] el0_svc_common.llvm.3148309083493694862+0xc8/0x114 [ 1265.554148] do_el0_svc_compat+0x20/0x30 [ 1265.554154] el0_svc_compat+0x28/0x90 [ 1265.554162] el0t_32_sync_handler+0x7c/0xbc [ 1265.554169] el0t_32_sync+0x1b8/0x1bc > This patch implies that _every_ driver with an ioctl must create a > compat_ioctl() callback, are you sure that is the case? > this depends on user application and kernel version. for 32-bit application to run on 64-bit kernel, compat_ioctl is requried. > thanks, > > greg k-h Thanks Viken Dadhaniya
On Wed, Jan 25, 2023 at 11:27:49AM +0000, Viken Dadhaniya wrote: > Hi Greg, > > Please find response inline. As they should be :) > > -----Original Message----- > > From: Greg KH <gregkh@linuxfoundation.org> > > Sent: Thursday, December 22, 2022 11:19 AM > > To: Viken Dadhaniya (QUIC) <quic_vdadhani@quicinc.com> > > Cc: jirislaby@kernel.org; linux-serial@vger.kernel.org; linux- > > kernel@vger.kernel.org; Mukesh Savaliya (QUIC) <quic_msavaliy@quicinc.com>; > > Vijaya Krishna Nivarthi (Temp) (QUIC) <quic_vnivarth@quicinc.com>; Visweswara > > Tanuku (QUIC) <quic_vtanuku@quicinc.com>; Aniket RANDIVE (QUIC) > > <quic_arandive@quicinc.com> > > Subject: Re: [PATCH V1 1/1] Serial: core: Add compat ioctl support That's horrible, please fix your email client. > > How was this tested? What is currently broken that now works properly (or the > > other way around?) > > > > We are running 32-bit user-space application on 64-bit kernel, and vendor specific ioctl command issued by application is not reaching the vendor driver. > Without these changes, the IOCTL call was returning from the tty framework rather than reaching to the vendor driver. IOCTL call is successfully reaching the vendor uart driver via tty framework after adding compat ioctl and functionality is working as expected and below is the dump stack with compat ioctl. What in-kernel tty ioctl is having a problem that a compat ioctl layer is needed? Let's fix that up. As you know, we can't do anything about out-of-tree drivers. Nor do you want us to, so this really feels like a broken driver, it should NOT be creating random new ioctls on the tty device node, that is NOT what it is there for. > [ 1265.554002] Hardware name: Qualcomm Technologies, Inc. Monaco IDP V1.0 (DT) > [ 1265.554008] Call trace: > [ 1265.554011] dump_backtrace.cfi_jt+0x0/0x8 > [ 1265.554023] show_stack+0x1c/0x2c > [ 1265.554032] dump_stack_lvl+0x80/0xc8 > [ 1265.554041] dump_stack+0x1c/0x2c > [ 1265.554049] msm_geni_serial_ioctl+0x2cc/0x3d0 [msm_geni_serial] This function is not in our kernel tree, so it looks to be a bug in that driver, sorry. Do NOT paper over out-of-tree driver bugs in the core kernel (hint, are you sure you are even allowed to do that?) > [ 1265.554099] uart_ioctl+0x1a4/0x1d8 > [ 1265.554109] uart_compat_ioctl+0x14/0x28 > [ 1265.554117] tty_compat_ioctl+0x1a8/0x2dc > [ 1265.554125] __arm64_compat_sys_ioctl+0x158/0x1d0 > [ 1265.554132] invoke_syscall+0x60/0x150 > [ 1265.554140] el0_svc_common.llvm.3148309083493694862+0xc8/0x114 > [ 1265.554148] do_el0_svc_compat+0x20/0x30 > [ 1265.554154] el0_svc_compat+0x28/0x90 > [ 1265.554162] el0t_32_sync_handler+0x7c/0xbc > [ 1265.554169] el0t_32_sync+0x1b8/0x1bc > > > This patch implies that _every_ driver with an ioctl must create a > > compat_ioctl() callback, are you sure that is the case? > > > > this depends on user application and kernel version. for 32-bit application to run on 64-bit kernel, compat_ioctl is requried. Again, what in-tree tty ioctls are affected by this? thanks, greg k-h
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index b9fbbee598b8..5ffa0798db3b 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -1601,6 +1601,22 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg) return ret; } +/** + * uart_compat_ioctl: uart compat IOCTL function. + * @tty: pointer to tty structure. + * @cmd: command code passed by user-space. + * @arg: argument passed by user-space. + * + * This function will call normal uart IOCTL. + * + * Return: 0 for success, Negative number for error condition. + */ +static long +uart_compat_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg) +{ + return (long)uart_ioctl(tty, cmd, arg); +}; + static void uart_set_ldisc(struct tty_struct *tty) { struct uart_state *state = tty->driver_data; @@ -2670,6 +2686,7 @@ static const struct tty_operations uart_ops = { .chars_in_buffer= uart_chars_in_buffer, .flush_buffer = uart_flush_buffer, .ioctl = uart_ioctl, + .compat_ioctl = uart_compat_ioctl, .throttle = uart_throttle, .unthrottle = uart_unthrottle, .send_xchar = uart_send_xchar,