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:   Mon, 4 Apr 2022 13:50:41 +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
Subject: Re: [PATCH V2] Fix unsafe memory access by memcmp
Message-ID: <20220404105041.GX3293@kadam>
References: <YkpdVyuQguGzPSNE@sckzor-linux.localdomain>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YkpdVyuQguGzPSNE@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?fgkeXo7C8B7vC5tAGERaIinqCiNTK33uD7m5TTVqwcMwr+GruAbmQYg9Cd0N?=
 =?us-ascii?Q?pFDS6OhFS0GM1NMf+ljM/bRnQh86S7nXV2+kaB51W4+L7T/0X0ejtxJSQe3U?=
 =?us-ascii?Q?j2dF9QX8OQbJH9nCGU4TWSVq+8RnR3tj+PWmii+k/u/F/XQ22Wp97Wp4fqDg?=
 =?us-ascii?Q?MQflwagQUQrcfutbvp/VCTYLj7ycKSaiyP0LFVRw0WvZjH8Qs5t9+W/9wBOJ?=
 =?us-ascii?Q?BfOAJakCR7uC23dlevlLJqA/LSgKEg4gNiswpi6PCHuMSyQ2GDHsbDFE98BR?=
 =?us-ascii?Q?0JmwxQSQO2i7KH5L1xBPXg4BxiS6lT3lacp6KeeGgV+RH9gDOW5pcIoNYkmR?=
 =?us-ascii?Q?AjxybluH6RKmq5QrAIqGCVESQsYoNHFErAjyLkG4zS+ELr74KO3oC5CQfs7v?=
 =?us-ascii?Q?BtlGkqc6oseWMGhNNh1lwNPTLRsdDI6T1rtSRo991by6nSWp/lOdbN0Lf0cC?=
 =?us-ascii?Q?HssJhqxo1Z4ceBLMGLj4t4/pmr9LIWpkK3qjKhh4wugZyW+v8E/EfV8dYH7w?=
 =?us-ascii?Q?7sCaeAvACXxRWv7bzw5V6JcXFTXc2F0D/Pma53eZY4XxOpkBBUd5bNw+X5Z6?=
 =?us-ascii?Q?IRrrokuBIG55EGcdOibwK6qgVP5dwKpfXd/Nprw/3CGQwEliHZ2kQReLrOnT?=
 =?us-ascii?Q?xHw22wgDF2zo/B0uHSnl21eXw5foXRxa3XfGdvUSFGjjm9hdCh27/OGXZaez?=
 =?us-ascii?Q?BQu2TPJLtU/+glZg8v+Kzraz7oegekTi6EdCHaGC3E8KvYGCa5XUHkWiQO4q?=
 =?us-ascii?Q?9LZMIHo68AFDnnPN0lIBE4oeuZ55DBElHjHu3j3QGOuToS/p3fVWACI4xT02?=
 =?us-ascii?Q?KkDcZY0g22jaPFSKM/79COCUeO5fL1p7ZGgAbJyXwGWLh4iU0yYgy01fDE+I?=
 =?us-ascii?Q?43OWfc60AtLExSbe2Pw+RukDLIKqWtfeuGVNuNv04+exbfxQ7JL4JbDkTyt3?=
 =?us-ascii?Q?GbUcG9SVBPkMB0hou/ufqpryP0mC5l6V2Xh9v+Fb7lQVKk0lh1wGaHJXyIGC?=
 =?us-ascii?Q?dtio2mj1kBJ4nObWT++zsbI8EUdA1udqzbjSuB9BGsLIW8WCdGLz6NtgClAg?=
 =?us-ascii?Q?SNKPswMcl55y6NNHiIYy0Ka/iP2S12KSf36rGUK71GM8UQZFVSw39NwcP3iF?=
 =?us-ascii?Q?DrYpneMS9rWbIQ9Kfz2xRjYopcCwECLrga/P1vRVxrZZDGChj90t9DB4UZry?=
 =?us-ascii?Q?kErgynY0pENceUYtF/Zq8dVtvGKl9jbBsTHMKyxeTH2oi/gQ9Ud5NUwVPlf/?=
 =?us-ascii?Q?2mWSYMBv7swZ8wdyP8gO/OzLKoj87ymMqC6A/4jUwHloBGPHEtcC4nzR7rlt?=
 =?us-ascii?Q?gRuoCLxHixzoiybtEBbQUnkvQ4C0+airslemkmnlXoFW9mb5HbylNKnwhGKo?=
 =?us-ascii?Q?t98JmTyh0xil8elNS8Iyh1xPZtVvdgFTDv/bx3mc7G9gsl5d+0Cjw3NJdvXP?=
 =?us-ascii?Q?ApFu+X+GjwNoWKIP7+vCUApaorKGE5RcVFSjqVAXBUpNag2shfwf4X9FA6v6?=
 =?us-ascii?Q?xPFaEl3JNYta4evoarqI/ywtoo7cJTY4U/cNU/Dw26DkDQmKMNaJNFUw7rDd?=
 =?us-ascii?Q?qovhZRdHbKP1kzfLXSR9+AbgpFw+yzSEiIeLH/h9Ep5KtlIqoYFH0hiMZH4C?=
 =?us-ascii?Q?kAG4IzuCrYdULX+wy0iP3Xzukh1+d2roEmKNNgUj/iIFTEd2dbAWDJRvz6Kw?=
 =?us-ascii?Q?lek1hVVReY8XMOPGJcfPeLZjRCqR7ggu0J2CXW7sfsPj88QfRQvADoXK92Zb?=
 =?us-ascii?Q?+gZwNddW/Amh6I/xwcxizixEFk1QML8=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5ea0ff85-c751-4144-0f7c-08da162901b4
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Apr 2022 10:51:01.0018
 (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: OMfVaTljp/GKGTlYG++XCeBx56mA6hTFJltNKqysk9u3JitQnGvyw9DgO2D2tDVm6b3eoYgyyQvDB2RnVeJ2qfv/6QbdaJkt/3jRA61ybt4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR10MB3187
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.425,18.0.850
 definitions=2022-04-04_03:2022-03-30,2022-04-04 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 spamscore=0 adultscore=0
 bulkscore=0 mlxlogscore=999 phishscore=0 suspectscore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2204040061
X-Proofpoint-ORIG-GUID: 6NIKwbmr5w3gv5nPd9TaTLHFa2-sAkZo
X-Proofpoint-GUID: 6NIKwbmr5w3gv5nPd9TaTLHFa2-sAkZo
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 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?

regards,
dan carpenter