Date:   Fri, 10 Dec 2021 11:30:10 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Cc:     Larry.Finger@lwfinger.net, phil@philpotter.co.uk,
        gregkh@linuxfoundation.org, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org, Abaci Robot <abaci@linux.alibaba.com>
Subject: Re: [PATCH] staging: r8188eu: Use memdup_user instead of
 kmalloc/copy_from_user
Message-ID: <20211210083010.GM1956@kadam>
References: <20211210033832.40428-1-jiapeng.chong@linux.alibaba.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211210033832.40428-1-jiapeng.chong@linux.alibaba.com>
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?mnAWUER6W8JGJ2UJLP9xbaB0rIrEi7bi/IsBi4RGRjlcDeMcCvs8Qr8yX9qM?=
 =?us-ascii?Q?ohkrp8davJIEiOKW6ydcmfM9mihy7G6MvYWLUhijj+Ep0Mra6zoOANZWTH/b?=
 =?us-ascii?Q?AtE0NwRUHL3xMc+Q9G9QL3cXAcy1o5cnGZkyxv0fSmd8WISxPMYw1hMFuhjq?=
 =?us-ascii?Q?H3klBlleyDrKdoao7BgeXd8alxDoYjzg6bGro2gGocTYXl0X1w+ppL6OgeSx?=
 =?us-ascii?Q?tSAoBAirEmqa0MInYK+MY/icttA8yNjcQLx6G1n1NXSgYEKjtgWvYTN2PpqL?=
 =?us-ascii?Q?hjHG48/MzjILw3d7ziiWOZuaCe2141NBkYUbzeOOp6lYQ/a93Ek+gCgpKE08?=
 =?us-ascii?Q?7IwYhxv7zWFxgxv9ry2fq37bX/LzdL9WeR7C4PwlnAQjEEcE5l3PYJc8fOKB?=
 =?us-ascii?Q?GodB7FW5iMxM7j2qkWDkwDwjTGrWcWPC7wh12hUecq+ToS52aCc3cRFLHHi6?=
 =?us-ascii?Q?pxnfkEDWrzqORNXm8TYTRN4d5F/pEA2qfysMq/X+t+dGu5rR40YYNxGfDi21?=
 =?us-ascii?Q?BHVVGpIlZHQ369uKT+AllngcuzaF2LFbIGfbQXOdJtnBElofHMI6z2kLoQIj?=
 =?us-ascii?Q?v532T7XT46qulqDunr4txW+u2X7Rcm7oZxtfKlqh17+YMOyBsxKFWVOdL4ge?=
 =?us-ascii?Q?eJk2O4Y2UcViqIX2D0Sk9dY47inBSZa5dDk+LPxHClnn/Y8mHcr8ODAJk/54?=
 =?us-ascii?Q?sJMKuainYoSG/nP4RDPcXTErypOVeQOV4j7rSwf2EvDXZmackXRbt7yOOBvx?=
 =?us-ascii?Q?ey4C69hEyuJNeUkB2PAmKTmB4EEbjA296QVZp6IubLw+i7Uqezc1yWyeaEqR?=
 =?us-ascii?Q?pGLEeND1hEP9X1MRrVK7tOGoZ8bpxKBU2Ux/lEfzsZyhy8WyVOOWFu4elFRf?=
 =?us-ascii?Q?aPG47DO+AEULZ/3pgtJei+B7BGrtkJ9URKgMUBSjHKzcRJlTmtSgsQ/eLCX3?=
 =?us-ascii?Q?l7d5tRgvd1fNQ+nAqIuJLZchbKzi7B37NpwMp75ocv4JV9y273f9oY5OgeQp?=
 =?us-ascii?Q?Dc9v/+qp8Iwqfsu2eKCrKL6eNX4qW/x3s3RGA6jpZVOH4U4TVgUsozTDa/e1?=
 =?us-ascii?Q?VRpv9BlEj2IcHim/3Gs1uRHN193gv43bCAUDyKcQh5A19q4N5rJlYDeas5lW?=
 =?us-ascii?Q?+ahvjbR/CMPrNP5fqRr2Zs4ujk8ldL27ItSQY8y0ooFbRnRVWkzAFb0vHIPt?=
 =?us-ascii?Q?nIqX3CzL347Y+z9/c0Qs/teSBSunkGhVjVKrvNIbuDfPgl1c9dB0x3fUu6OL?=
 =?us-ascii?Q?2+dSak+dDHsIi4iWSPthRTr6B1OdVhNPeEOFxgd1/86fkD4f95ftCl45CQrp?=
 =?us-ascii?Q?b9f4kcDmKIbOVivjhr6vBJwbYMH5v3XP86I0YQvylvpUF9vFx3/MAs5oymF1?=
 =?us-ascii?Q?jtq9eYvQ9n8SJT54JDSMWUPk4F8WQ1IhhgrIa1f07kfETQQb9AgwfX3l3gJX?=
 =?us-ascii?Q?xUUd4A0qtmNufxdz2t49IqvZTj5FaiCGQZ6/vICkAH3WIWMOjC5fDE53Lefz?=
 =?us-ascii?Q?x6cDtbJvF5CyNSCyypS2a3ttBzU+cs4hdqVyXoJwF4PWTr3lzVHOnsZPYxlq?=
 =?us-ascii?Q?7wgI+92aMYNTB5sPtO7z4/xFfKNDmqcWeVY5bKBbuFL4V2dI/CObDxdLzAr/?=
 =?us-ascii?Q?qqCtl02DTxDtLtcldsV767PRR+wI92QDIASzZ7un+ybpJCNST1A0s6vUkShQ?=
 =?us-ascii?Q?s7Vv9VFtjMvJjX/Th4h9A1/x/ac=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 248ebf93-aabf-4b7a-9639-08d9bbb74f31
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Dec 2021 08:30:23.9720
 (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: ExbOEowaRKbZld4sBRoREuJXBKPNuy3UC6onUf54HlC0/B1ORqS5wHlmkguy0eAlWOzK9gfuo5xoY5hUAdlnQqkKSwbEVKhEmhCUfKhoHrU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO6PR10MB5792
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10193 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 mlxlogscore=999
 phishscore=0 bulkscore=0 malwarescore=0 spamscore=0 suspectscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2112100046
X-Proofpoint-ORIG-GUID: -yjiSGDbMVvQje9oi3TtMQT_TBtM3-4r
X-Proofpoint-GUID: -yjiSGDbMVvQje9oi3TtMQT_TBtM3-4r
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Dec 10, 2021 at 11:38:32AM +0800, Jiapeng Chong wrote:
> Fix following coccicheck warning:
> 
> ./drivers/staging/r8188eu/os_dep/ioctl_linux.c:4253:8-15: WARNING
> opportunity for memdup_user.
> 
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> ---
>  drivers/staging/r8188eu/os_dep/ioctl_linux.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> index 56adfe4087a8..c6a2fbfe8f26 100644
> --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> @@ -4250,17 +4250,12 @@ static int rtw_test(
>  	DBG_88E("+%s\n", __func__);
>  	len = wrqu->data.length;
>  
> -	pbuf = kzalloc(len, GFP_KERNEL);
> -	if (!pbuf) {
> -		DBG_88E("%s: no memory!\n", __func__);
> -		return -ENOMEM;
> -	}
> -
> -	if (copy_from_user(pbuf, wrqu->data.pointer, len)) {
> -		kfree(pbuf);
> +	pbuf = memdup_user(wrqu->data.pointer, len);

This code assumes that the user is going to give us a NUL terminated
string which is not necessarily true.  The original code was buggy too.
Anyway, please fix it.  Use strndup_user() and mention that it is a bug
fix in the commit message:

	pbuf = strndup_user(wrqu->data.pointer, len);

Add a Fixes tag.

regards,
dan carpenter