From patchwork Fri Jun 3 09:24:12 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Prasun Maiti X-Patchwork-Id: 9152367 X-Patchwork-Delegate: johannes@sipsolutions.net Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id CC63160751 for ; Fri, 3 Jun 2016 09:24:30 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BBCE1282E8 for ; Fri, 3 Jun 2016 09:24:30 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B04CD28309; Fri, 3 Jun 2016 09:24:30 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A4283282E8 for ; Fri, 3 Jun 2016 09:24:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932418AbcFCJYX (ORCPT ); Fri, 3 Jun 2016 05:24:23 -0400 Received: from mail-oi0-f67.google.com ([209.85.218.67]:34792 "EHLO mail-oi0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932393AbcFCJYT (ORCPT ); Fri, 3 Jun 2016 05:24:19 -0400 Received: by mail-oi0-f67.google.com with SMTP id e80so8318871oig.1 for ; Fri, 03 Jun 2016 02:24:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=eSXlQ2PKW3fMi+GDFXkPFbtylOGeOldSgT67xYyQVFM=; b=yQYE9ZQdYioUGWuUfdFbkVMz1refx3fXroHjza2iUE4mUIwvlFXNkSEjE7uEXVihAV JoWMJ0b/IsN2l0CZjWjKhVHtfNmZvO0WxkgVxKtuCOqiD3rrB29Z8YTtXaEMiLde2oXR BuirBYgeS5G2yMbzGjA57KbVKQ2exp0ahV/8/+UICR/dQTTwmJIk8jDpiEVLG4YKWPGR rPnN8CPXoHRWFDNl4gKxw2aXQEnuzSWka0TLPreR9kz+W08J3vrKA8HsNYf6whSbS71j mChJa1c016GPZIu5aE18rlI2Cvm4Kt7PkAoVC69003WvjQAw9LAi9bco3uwFvT+Lx64x qMnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=eSXlQ2PKW3fMi+GDFXkPFbtylOGeOldSgT67xYyQVFM=; b=Y0EXgWoXex1YIj9wlhGCgtU/6w3blvK+Vf1QjkTgtypVC9ZjT27GWk90PRGMBHodIX uzQeNac7ePkjc81FV+ijt60ZHGFo2Gdsdof3tzOH/J65YaPGu6ticJTKf0sTFnWCBdX8 fYc8hmZT5+GYjlN5RByEbMutg8MKtVvFFmjjGaw3keaiD4w684YZjgEfjBmdlhv6MUeh 7CETY5o//srMRfluf1Czx48+Pm1ZzFb1nMJIWH/f1MJZxWnlx3WC0fNAxLJef2d1JG5p ICZSwxbJ8THUl1dIGeov/sJ4jWrqRhgcuuPDC96MazHgeJLF/OSGHO2eej6BXqSmgnSd +3rg== X-Gm-Message-State: ALyK8tJu0Vl5QIKH4rf72EWjX/8xwiV3Xx25yPy4mHpHqUfQU0AMlT9wHJNtrrxytFCfxOpZFVjeF1PncFnPPg== X-Received: by 10.202.168.131 with SMTP id r125mr1311934oie.40.1464945853219; Fri, 03 Jun 2016 02:24:13 -0700 (PDT) MIME-Version: 1.0 Received: by 10.202.198.16 with HTTP; Fri, 3 Jun 2016 02:24:12 -0700 (PDT) In-Reply-To: <1464690345.3076.17.camel@sipsolutions.net> References: <1464248432-12595-1-git-send-email-prasunmaiti87@gmail.com> <1464690345.3076.17.camel@sipsolutions.net> From: Prasun Maiti Date: Fri, 3 Jun 2016 14:54:12 +0530 Message-ID: Subject: Re: [PATCH] wext: Fix 32 bit iwpriv compatibility issue with 64 bit Kernel To: Johannes Berg Cc: "David S. Miller" , Dibyajyoti Ghosh , Ujjal Roy , WiFi Mailing List Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi, If a wireless driver uses "iw_handler_def.private_args" to populate iwpriv command for private ioctls but handlers for private ioctls(iw_handler) of iw_handler_def.private is set to NULL like "atmel" and "airo" wireless drivers. For those case, the IOCTLs from SIOCIWFIRSTPRIV to SIOCIWLASTPRIV will follow the path ndo_do_ioctl(). Accordingly when the filled up "iw_point" structure comes from 32 bit iwpriv to 64 bit Kernel, Kernel will not convert the pointer and sends it to driver. So, the driver may get the invalid data. We have used another iwr (i.e; iwrp) to pass things to driver. Changes are as follows. Please have look into this. diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c index 6250b1c..50583fb 100644 } Is it Ok? Thanks, Prasun On Tue, May 31, 2016 at 3:55 PM, Johannes Berg wrote: > Hi, > > I must say, this is a bit of a surprise - where is iwpriv actually > still relevant? What driver could this possibly matter for? > > Anyway ... > >> + if (dev->netdev_ops->ndo_do_ioctl) { >> + if ((info->flags & IW_REQUEST_FLAG_COMPAT) && >> + (cmd >= SIOCIWFIRSTPRIV && cmd <= >> SIOCIWLASTPRIV)) { > > This has coding style issues, obviously. > > Also, handling the non-compat case would allow you to return and reduce > indentation by one in the large code block that handles the compat. > >> + int ret = 0; >> + struct compat_iw_point *iwp_compat = (struct compat_iw_point *) &iwr->u.data; >> + struct iw_point *iwp = &iwr->u.data; >> + __u16 length = iwp_compat->length, flags = iwp_compat->flags; >> + >> + iwp->pointer = compat_ptr(iwp_compat->pointer); >> + iwp->length = length; >> + iwp->flags = flags; >> + >> + ret = dev->netdev_ops->ndo_do_ioctl(dev, ifr, cmd); >> + >> + length = iwp->length; >> + flags = iwp->flags; >> + iwp_compat->pointer = ptr_to_compat(iwp->pointer); >> + iwp_compat->length = length; >> + iwp_compat->flags = flags; > > Why don't you just put another ifr/iwr on the stack, and use that to > pass things to the driver? This modify-in-place of 'iwp', which > requires loading all the variables first, seems very awkward to me. > > > johannes --- a/net/wireless/wext-core.c +++ b/net/wireless/wext-core.c @@ -958,8 +958,28 @@ static int wireless_process_ioctl(struct net *net, struct ifreq *ifr, return private(dev, iwr, cmd, info, handler); } /* Old driver API : call driver ioctl handler */ - if (dev->netdev_ops->ndo_do_ioctl) - return dev->netdev_ops->ndo_do_ioctl(dev, ifr, cmd); + if (dev->netdev_ops->ndo_do_ioctl) { + if (info->flags & IW_REQUEST_FLAG_COMPAT) { + int ret = 0; + struct iwreq iwrp; + struct compat_iw_point *iwp_compat = + (struct compat_iw_point *) &iwr->u.data; + + strncpy(iwrp.ifr_ifrn.ifrn_name, iwr->ifr_ifrn.ifrn_name, IFNAMSIZ); + iwrp.u.data.pointer = compat_ptr(iwp_compat->pointer); + iwrp.u.data.length = iwp_compat->length; + iwrp.u.data.flags = iwp_compat->flags; + + ret = dev->netdev_ops->ndo_do_ioctl(dev, (struct ifreq *) &iwrp, cmd); + + iwp_compat->pointer = ptr_to_compat(iwrp.u.data.pointer); + iwp_compat->length = iwrp.u.data.length; + iwp_compat->flags = iwrp.u.data.flags; + return ret; + } else { + return dev->netdev_ops->ndo_do_ioctl(dev, ifr, cmd); + } + } return -EOPNOTSUPP;