Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Wed, 6 Apr 2022 16:14:26 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Charlie Sands <sandsch@northvilleschools.net>
Cc:     gregkh@linuxfoundation.org, Larry.Finger@lwfinger.net,
        phil@philpotter.co.uk, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org, paskripkin@gmail.com,
        straube.linux@gmail.com, fmdefrancesco@gmail.com
Subject: Re: [PATCH V3] staging: r8188eu: Fix unsafe memory access by memcmp.
Message-ID: <20220406131426.GH3293@kadam>
References: <Yk14tpZCOFCsvtup@sckzor-linux.localdomain>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yk14tpZCOFCsvtup@sckzor-linux.localdomain>
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?tZFnZpZEJgJUY6rj68HmrYQIBDyYszXQOqFiyw1tmBuzP81wvaWC9abcp408?=
 =?us-ascii?Q?M/eIrHKqxYzxo4Ekawy3iwac8g1/JGQZeVXiS95j4D9aVKn4NJQqsKLSeKgE?=
 =?us-ascii?Q?UkxasmCWWX3wtqdlcHRAnCaOTfr1ASYrx/lS1HyLGwWm8p+8s4Ag8IzAp0W+?=
 =?us-ascii?Q?iNX8Rr4wRFCSBVK90PMNUazjGGYMB01ddEPRLARNP/P0baYVjqhQelLZKKHK?=
 =?us-ascii?Q?lmt2Khwl5W87uqiRYR+tkrCsgu4SFxPVAq5nBJBV+UrW/dE8ZwWdVVLpRcDe?=
 =?us-ascii?Q?foLvP3jQPxyESy0U8tCnnzyX33mn+x42wkL2wnnOv0h1/7Gu/tahaphPDjM3?=
 =?us-ascii?Q?TI8J4iwbcX9Rm9ieLAcLVxeNwTB3aDQos8YJg9pV0tMR08pJIcovzo9fCyjz?=
 =?us-ascii?Q?WYzrBoDWILlsBXQBu5qD3Ijte5J8J+tDHamnbgKQVex9ustIeGfSwDcikQc3?=
 =?us-ascii?Q?yq/4VUm7MlSBlrICDAWI+6Wx3v+B0aZ7oWmZVOV7uuqSr2A0Pg9uTDz6Ng7y?=
 =?us-ascii?Q?J3Ik7p9dLwR7XX2RG56CTz8nSOhYqHox9chG2v4wpBZfCyEo3BFGTRnyrYlR?=
 =?us-ascii?Q?wPvor8EqP4iqYnmYMllV1RMNh9sK0jwX7m0dS5oidzI2yH3DsEXEHt3aFhEo?=
 =?us-ascii?Q?sqxgoUus5ZNJjDuYS3XywZYhV47f6QHb/2Gd+/+FnZWwTLpHCFsN8E7nEvdd?=
 =?us-ascii?Q?3RypEM5mIqGild15TXsnLnOojN40Rh+Hl/2X+/li/+XWjAVYunfvKwgAkAG9?=
 =?us-ascii?Q?IjRZci82GtmrbXm5tiUOWudnRmmoLMnu3hwJ0W7FVfAy5lzcRaXH4JP0jEMX?=
 =?us-ascii?Q?DPcIj8VVqady3n/AYRtPA1qzp/KCM2FzaECnsayGKKZkkh0CfVclwRFMMKte?=
 =?us-ascii?Q?3F0ttvwkcMiQlll5e5fnOVvWqfpdeJa9fn6H3oppAoJcPcoZueCeQcefcUiF?=
 =?us-ascii?Q?SERSIAfsPqRslwXkpYl8QEzL17ZIvgEqrQauCxcgV0SuL6t74s40JWJSXwDQ?=
 =?us-ascii?Q?dWObgcNF3i5HmJ6ELJSrPUMmag/kMQ5C6PiTJAj+Hmce8mFbSQ/NmImQQvIT?=
 =?us-ascii?Q?FY7wWKsIfIPKElEVVx13WPufkQBtLKiuhJB34c/IeKhm8CDMZPnqe2mACpii?=
 =?us-ascii?Q?NJFne40hHi76JJadjY+evDz0/o6bRuV/nGqWXO6quh3EB8/Afh48M57CxP3C?=
 =?us-ascii?Q?p37I3xRQCqLo4BHL3Z9Evd67vJzF5Dwdpb3K7g+D+hqWJpEm2TpIaFAEIxpS?=
 =?us-ascii?Q?HFgCjIDuvUJFBvzNlkTiogxq18lUWvKIssZzz4gIh6aQvXKnzV/oqLv6+dj/?=
 =?us-ascii?Q?ClLaY25GPUB4nSrFJ0epmkQsuwA2OxqFEqhgt3eSX8Ssr6Q6BiADFZpAFr2b?=
 =?us-ascii?Q?ZYENRM8bstzTk2RwXtOCj86Acd46ZMy/4YlbleK4kdPtwH1gSUStPC1ziqzM?=
 =?us-ascii?Q?GNlg1OMlM2JUoIPiXvWQhgL3Uc7HO3XpCx7nm5SUx4l4X7f5xOtNt4a5Gc29?=
 =?us-ascii?Q?2OKAjgCTkLOAxBgh/+XS6+8KaxG6oolojhLYHnhjH+th46EDSpX6Ja9P3vrR?=
 =?us-ascii?Q?44jt8bXmtHUp3PcJ6+2ENIU2apmzCvxkhMaWxhej+kmobnu0OriXPkhRYN5P?=
 =?us-ascii?Q?WgZ6P8uqsGqY8G2DCfgp6be4J9qnLKtYBAmvXG6+TNM8+f7CAp0Qnju7P4nQ?=
 =?us-ascii?Q?7TV6jyjm/UzKufVVNygLXUzFY/vUBMRMQ/qtcYtE4gwRanUWuUlahyF/PglG?=
 =?us-ascii?Q?nCZCPxozcDlBjo2qFQGliJFEqhWdRRA=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 59704edb-ee97-46a9-a3cb-08da17cf6bf8
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Apr 2022 13:14:47.0659
 (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: bjGZiDslSA3UiWsNQlrY8cJlKGj27VoAD+V3vAl88AvBdU4lKOLbdmT264guYYYeY/MDFGupMtv/4AHFlN2feJZ30LEBmZst5KmD2ZwbTQE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR10MB1737
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.425,18.0.850
 definitions=2022-04-06_04:2022-04-06,2022-04-06 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 malwarescore=0
 mlxscore=0 suspectscore=0 bulkscore=0 adultscore=0 spamscore=0
 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2204060064
X-Proofpoint-ORIG-GUID: NmFGL-lqybjNlrWA2lxcG82TnKZXHQ5H
X-Proofpoint-GUID: NmFGL-lqybjNlrWA2lxcG82TnKZXHQ5H
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 Wed, Apr 06, 2022 at 07:25:42AM -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.
> 
> V3: return -EFAULT from the function when there is a memory error while moving
> it from user space to kernel space and fix incorrect subject from V2
> 
>  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..9f2182b8e451 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)

How do you know that the user buffer is passing 9 characters long?

You're fixing a real bug, sure, but you've inventing a new API and it's
a rubbish API.  Just delete rtw_p2p_get() and the associated code.  It
has never worked so no one will miss it.

regards,
dan carpenter