Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 15 Apr 2022 08:25:02 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Haowen Bai <baihaowen@meizu.com>
Cc:     gregkh@linuxfoundation.org, davem@davemloft.net, len.baker@gmx.com,
        dave@stgolabs.net, edumazet@google.com,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH V2] staging: rtl8192e: Fix signedness bug in
 rtllib_rx_assoc_resp()
Message-ID: <20220415052502.GE12805@kadam>
References: <1649988571-19923-1-git-send-email-baihaowen@meizu.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1649988571-19923-1-git-send-email-baihaowen@meizu.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?0vuEvtMZ3Wngh/M6ujhz7dz7kGjYn8uvLJh2q2JNSdCsdkqo7VK/U0ZVsJYL?=
 =?us-ascii?Q?YdHdUmAJU2POfuzwKgRvB1jYrqx/n3eSInBNL6ZNE8iItbJIifV1tqVyuXTc?=
 =?us-ascii?Q?shiy73WxS47e+zfhic7QtY6XtRCZgSNHUDNJZBQ4+61fpo2RE8BDvRqUmVN9?=
 =?us-ascii?Q?2Cs6A0Zf6Ww91ZumsoXfvQspVPJhOa9D+wBxd+8glw9ipzNueYnRIBEPBFSn?=
 =?us-ascii?Q?458KXiPrrFUvSIRpNW6dH8kpZ+AhCZPlCsYz9oBC3ho6E6f0R6y0D8hthjBk?=
 =?us-ascii?Q?zt1LPBpc60ipNMvNbvrB+LetXkLilHwETz8MAMjC5qirr1btl0C5daPUdKXA?=
 =?us-ascii?Q?MLEzPexWR/nuK2sWYXh3iCkK4wc+LBYkRHgLTDPMc+hptNSlHnkwcO4TkUUr?=
 =?us-ascii?Q?rWfr3hH9RUY1D4HlnUuin0SxGtvABwp6+Kz4Socm7fe4J/5uRuA14tY5zWxg?=
 =?us-ascii?Q?kNoAwS3jqC/qZVczWgVWJvjG317osEP0sv7Ka3N2LphLcQqy+fS/+vV36D2T?=
 =?us-ascii?Q?Z8SIWlMTS1XsdHrL2c2t4xKqp/rCaMWuo+WbZV20kSZkMiwAaF/A155LQ/Yp?=
 =?us-ascii?Q?1JMxfiCyHd/OG+/KiRx15bmhb5kDB00jcZcH8zEyJBITtHwI2yvUo4urCf6f?=
 =?us-ascii?Q?CKzN4V3pGBCJfmChOtEHBIHol4D4XPPkxKVBNAdNYOM1cp5J1uAGKxWQKwrS?=
 =?us-ascii?Q?3EhWcvMlz3m/nVWfnK34rj/OdgYemXFi86IKBemYiswkvHxy8Q+/rhahr+Pq?=
 =?us-ascii?Q?I+yZFDf+IRicTSGznAb5YYZCAbjHnlE0F48FVYDN+zP8i0MJW8lQL43EaTG4?=
 =?us-ascii?Q?R0y/wIOfBWODRh2P0EjZ7rIorc9sB34Ye0jye3LuPF9gryIKGsuIFB4lrmod?=
 =?us-ascii?Q?bpnW/2Rj/T2cnlGZbKv85bzI0MEekGCnxqmKXWlq4FFazieCcCbFIYMUcVf3?=
 =?us-ascii?Q?AVFyVu/CmVF3p2+lZbF4pj2aU/lLQCJHObvONkIpW5qcJzZZfmLVxHPi80Wc?=
 =?us-ascii?Q?4BC3sFgR4XzrR/SESly6vruwnZWpXXAWTQN1p5p9kdAuSTS3MP6fgkik8ROz?=
 =?us-ascii?Q?O3BatETIBf0vuqie0PlK1EG2QsgU7T/rAnVx1CAIz8l+EeG4iWgIIhRkNnPr?=
 =?us-ascii?Q?ZW71oX7spBsfzo8zwTfB1pgluKP7HAoDsyFcbmxDKbWgLDJEENzECt3Uy1t/?=
 =?us-ascii?Q?mVBt0U3bQkFyzyeVwpJDKTC6HCS9TKTMp+WQZQiFzUUAhXtyf/+BR2k1N/EG?=
 =?us-ascii?Q?60ug034AAcxTBi+Utxx/0uBetTj56LOtcphTTV2xqeFN5KVxwrFH2eIWw1Tl?=
 =?us-ascii?Q?7ojCohvz/mOzMHV+oUTbc+jxG3R6mehTv1TI6uIWL1PsAVPY9WWJq3Jku4qh?=
 =?us-ascii?Q?b02GnUDiCg4JiHMB/TTW9TC79iX8Yh4H1PZ+n9tbZUBbg4w0H5A3s6u/Ygyw?=
 =?us-ascii?Q?iAFvFYxa++/5M0yV6OF7sKhaS+vV2qScmwtNPQszzlUAtZeLgLVukOpiqcL8?=
 =?us-ascii?Q?PIgiZHXwY32hLPYWTAhvTVItyq4Bs8xEZU0OCGD2howCoJMPHttJ20FV8fJh?=
 =?us-ascii?Q?oiUyArGW+D16WYGjEfwJ6t/pRTyBezo6JlXgfaGKgpVghOhPFXCq7l2yN+/m?=
 =?us-ascii?Q?aIHG02DaegO6p3BrWjLTSCtFrm1mvCDJi0NIxMsI6XUTX1/Qe4M4XG6/IqfL?=
 =?us-ascii?Q?mXGLIKdlt2LEMbcvXBnWyfeQHUGCTyVSOzQd71ww0tpH0E31W0KscXhjMolT?=
 =?us-ascii?Q?yegw1NhEIzp380M2hE4bqfA6401HX6Q=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: db7194ab-fd79-497a-b372-08da1ea057c8
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Apr 2022 05:25:24.4592
 (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: CuB/OFNzWkKKCRxUpAH6EVfivrhxHKDE/6SRQMkkc38bRhDsmhCa1smNwCdTNOj6CuyWkaKsTMscfnWNRs1ldAV/7RESPIWoowm0+jTQwKU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR10MB4483
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-04-15_01:2022-04-14,2022-04-15 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 suspectscore=0
 malwarescore=0 mlxlogscore=999 spamscore=0 adultscore=0 phishscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2204150032
X-Proofpoint-ORIG-GUID: 7ucyCvKkeub4UH_y0nTaYbvgXSmGUZns
X-Proofpoint-GUID: 7ucyCvKkeub4UH_y0nTaYbvgXSmGUZns
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham 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 Fri, Apr 15, 2022 at 10:09:31AM +0800, Haowen Bai wrote:
> function rtllib_rx_assoc_resp () unsigned errcode receive auth_parse()'s
> errcode -ENOMEM.
> 
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> ---
> V1->V2: reduce return random value; print its own error message.
> 
>  drivers/staging/rtl8192e/rtllib_softmac.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
> index 82bf05eb1cbf..8a0961e64a8c 100644
> --- a/drivers/staging/rtl8192e/rtllib_softmac.c
> +++ b/drivers/staging/rtl8192e/rtllib_softmac.c
> @@ -1764,7 +1764,7 @@ static void rtllib_softmac_check_all_nets(struct rtllib_device *ieee)
>  	spin_unlock_irqrestore(&ieee->lock, flags);
>  }
>  
> -static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
> +static inline s16 auth_parse(struct net_device *dev, struct sk_buff *skb,

Could you make this an int instead of s16.  s16 is always a bit weird.

>  			     u8 **challenge, int *chlen)
>  {
>  	struct rtllib_authentication *a;
> @@ -1773,7 +1773,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
>  	if (skb->len <  (sizeof(struct rtllib_authentication) -
>  	    sizeof(struct rtllib_info_element))) {
>  		netdev_dbg(dev, "invalid len in auth resp: %d\n", skb->len);
> -		return 0xcafe;
> +		return -EINVAL;
>  	}
>  	*challenge = NULL;
>  	a = (struct rtllib_authentication *) skb->data;
> @@ -1787,7 +1787,7 @@ static inline u16 auth_parse(struct net_device *dev, struct sk_buff *skb,
>  				return -ENOMEM;
>  		}
>  	}
> -	return le16_to_cpu(a->status);
> +	return a->status;

But then just say:

	if (a->status) {
		netdev_info(ieee->dev, "blah blah failed");
		return -EINVAL;
	}

	return 0;

If you look up what a->status is, it can only be
WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG which is not worth preserving really.

regards,
dan carpenter