Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Mon, 4 Apr 2022 15:03:32 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
Cc:     Charlie Sands <sandsch@northvilleschools.net>,
        gregkh@linuxfoundation.org, Larry.Finger@lwfinger.net,
        phil@philpotter.co.uk, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org, paskripkin@gmail.com
Subject: Re: [PATCH V2] Fix unsafe memory access by memcmp
Message-ID: <20220404120332.GY3293@kadam>
References: <YkpdVyuQguGzPSNE@sckzor-linux.localdomain>
 <20220404105041.GX3293@kadam>
 <4729027.31r3eYUQgx@leap>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4729027.31r3eYUQgx@leap>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?pGSkYQ3TqaHCpC4s+TiwgpcVRtFlwjoCGBvm1ikESbUCBKDeOOJbCCy0FZbT?=
 =?us-ascii?Q?G+q1QEuiYhV9mzc31ncwc/f88Ko8ATwxH+RsW0CjvqHQzySedJJpteyTgM/F?=
 =?us-ascii?Q?vrjdDI7WIa/pmWuYmEI92AO/DAsvBE0GyPX6kDDwYWMa3JWDCbChIyTtAsgl?=
 =?us-ascii?Q?19yTzVnN0b/a5Fxdp0q9xh+TI3yqhTPF/n9TLf4qZQOt1ONmUGV6c3uuAZaE?=
 =?us-ascii?Q?2HU6cfOh8Ielc3rCc9cbmV9waTnMTm9e9VHtEYSU663oRh3uobAGWuCnI72D?=
 =?us-ascii?Q?ACRYInM0vQ3QNQVOvZUHGEyaIRfkJAM2RlQHUtQ9eYqqy0124gq0OzCDC0rs?=
 =?us-ascii?Q?g+J/2CdWNo8YeQpbiZ+yGyNT0dvBUBlG3og7Wh2lJaVNyyKEj0V2dVmchLLz?=
 =?us-ascii?Q?YaQw5LKY6ErGsqltP7aCVz18PSm1m0aMnH5ali56lebvQ13YlRbauwD1/vbY?=
 =?us-ascii?Q?+bdj+xiRbfRSYNm77d/lJOxgzPUzKr+NGuh7a9OVxGLPrHpqGOrQ1rgaiUJI?=
 =?us-ascii?Q?wGAW+jpnlhR+GCiJlz865vExUkGRZPvYSBhyqWGuIK5DozPknTQr7CGukp5U?=
 =?us-ascii?Q?pIxNSTsv3FEt+bURZrl6TvMntuLJReRv6OYLkrTxOfNfVwx+ZIHGYhVg6CFb?=
 =?us-ascii?Q?fdq6uhmPZFNthNhPDS6awCJ1Odz0he4AuWZsWPcZsvkeXu5ZkdITKWMZZ0kx?=
 =?us-ascii?Q?XZGHwJ6QP0s3W5HXbYNUOu/huzwpUj84/7ANKxUtG01cIf5OGJLJOlyfbHOO?=
 =?us-ascii?Q?UiBz5aaUs97sb9hxHwfj9Iw2+DgtpEO86DrKSk5WSXgKsC38zgNOGSKRUbHU?=
 =?us-ascii?Q?0qMf/cFFLTawfVQ9dugFMYH8jZaqu+oY+HvIeueblOCMG0cZOS/dTsrcZUvy?=
 =?us-ascii?Q?DQyvU6SD739VpAt7+dkMq8qE5J+30SXkEGIofKMaJXfE4+DrN3WhS+HcKiTS?=
 =?us-ascii?Q?QeffTDjr29ebGIlmN8vkAcOWEUSaqfAWoRxs6U8+hf9PM17s5wYvp3JuYsJf?=
 =?us-ascii?Q?AF0qZ2jf8uX2NEMKIAm1e+TxmEs+2FV9yIbGEATIF5JnzpuUMMDWfeYFYw+Q?=
 =?us-ascii?Q?oiUrIzy/XGQpI1VnX4J4cSuDMOkJflBRr/b8DQ7YKqN9jawWS2yJD5d35qXx?=
 =?us-ascii?Q?ySJGd1xrgQYioR4jsDCUigeQNKABI1k4tzwgC0gpPZcaP9Np4DNGGaynOjqH?=
 =?us-ascii?Q?NZUsJJXf/gwXg0BodnbpmChIBUCqHwaWRhiddquoMlXSAmg7KsPzGWDSkliA?=
 =?us-ascii?Q?Z4jxG1S+gDaZfudFw3nsiK9PCnxjuwhGak7dz8zCvITS2GCW5WPzCsI5cYCt?=
 =?us-ascii?Q?GYKrM+WqLfjlZMt/c+GC05mAr/2U6WVDblXUbS3HAr41hmMkgjNCBsHQiZ/B?=
 =?us-ascii?Q?gdzN+Vov6oGpjSqEVOlGQe0QynR7Y98XMmJcZur0lJzghBrDdH6d6b5xdlt8?=
 =?us-ascii?Q?Owa84OlWlgXUXQqt7x3LclqM6ZjTB6YtQ880iT4At/Y1vGjdUKigJkThTmUK?=
 =?us-ascii?Q?RTkzRKkkZMLrI75NRkPALaZ99IoU9/SfkirqZLFXn+NmP8rS5lkVPqAuGTZq?=
 =?us-ascii?Q?HlQVZuJb+RA6eMskapCdzuZ0VCup1pu0jHcvsfh7NgjKRECaIipZlK51zv2T?=
 =?us-ascii?Q?rz/16T5Rs1LdLqCly3V+yy7p/tppxA5BTCHfLYfv2raYxF4K901Qk33kPfaE?=
 =?us-ascii?Q?TopD9a7FyBMaEn15wrMDU7kydXeMoIdqT/enzevol6blanA62o3uY4kCRy37?=
 =?us-ascii?Q?ZfvSLXsOzfnZRjoodiktmL1xfV7fD0g=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a08a727b-3128-4924-e441-08da163328eb
X-MS-Exchange-CrossTenant-AuthSource: CY4PR1001MB2358.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Apr 2022 12:03:41.4537
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: G2UYVhDMwSbAygsuAMILhGzmtxbDcgRkgVkTdFDLtg/rNBU+qc/PrEhraHZVN7rSjhZxw3pt7thrCRjYRg4kvbZkctQ/8LqgxIfif78D+lI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB5243
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.425,18.0.850
 definitions=2022-04-04_05:2022-03-30,2022-04-04 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 malwarescore=0 spamscore=0
 phishscore=0 mlxscore=0 mlxlogscore=999 bulkscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2204040068
X-Proofpoint-ORIG-GUID: N5y3o5qrB8JYDM55pjYUFdgqAYcq-9Zq
X-Proofpoint-GUID: N5y3o5qrB8JYDM55pjYUFdgqAYcq-9Zq
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE
	autolearn=no 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

On Mon, Apr 04, 2022 at 01:25:37PM +0200, Fabio M. De Francesco wrote:
> On luned? 4 aprile 2022 12:50:41 CEST Dan Carpenter wrote:
> > On Sun, Apr 03, 2022 at 10:52:07PM -0400, Charlie Sands wrote:
> > > This patch fixes sparse warnings about the memcmp function unsafely
> > > accessing userspace memory without first copying it to kernel space.
> > > 
> > > Signed-off-by: Charlie Sands <sandsch@northvilleschools.net>
> > > ---
> > > 
> > > V2: Fixed checkpatch.pl warning and changed variable name as suggested
> > > by Greg K. H. and improved error checking on the "copy_from_user" function as
> > > suggested by Pavel Skripkin.
> > > 
> > >  drivers/staging/r8188eu/os_dep/ioctl_linux.c | 21 ++++++++++++--------
> > >  1 file changed, 13 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> > > index 7df213856d66..4b4eec2bde96 100644
> > > --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> > > +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> > > @@ -3233,23 +3233,28 @@ static int rtw_p2p_get(struct net_device *dev,
> > >  			       struct iw_request_info *info,
> > >  			       union iwreq_data *wrqu, char *extra)
> > >  {
> > > -	if (!memcmp(wrqu->data.pointer, "status", 6)) {
> > > +	char wrqu_data[9];
> > > +
> > > +	if (copy_from_user(wrqu_data, wrqu->data.pointer, 9) != 0)
> > > +		return 0;
> > 
> > return -EFAULT;  We can't assume that that user wants to copy 9 bytes
> > especially when they're passing a 4 character + NUL string.
> > 
> > This is a custom ioctl.  Called from ioctl_private_iw_point().
> > 
> > I think if you try to dereference a user pointer like this then it will
> > cause a crash, right?  So that means no one has ever tested or used this
> > code and we hopefully we can just delete it?
> 
> After a quick look, I'm pretty confident that we can also delete 
> rtw_p2p_get2() and rtw_p2p_set() unless I'm overlooking something.

What are the problems with rtw_p2p_get2() and rtw_p2p_set()?

regards,
dan carpenter