Date:   Tue, 7 Mar 2023 09:42:46 +0100
From:   Simon Horman <simon.horman@corigine.com>
To:     Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Cc:     Dan Carpenter <error27@gmail.com>,
        Alexander Aring <alex.aring@gmail.com>,
        Stefan Schmidt <stefan@datenfreihafen.org>,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Marcel Holtmann <marcel@holtmann.org>,
        Harry Morris <harrymorris12@gmail.com>,
        linux-wpan@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH next] ca8210: Fix unsigned mac_len comparison with zero
 in ca8210_skb_tx()
Message-ID: <ZAb5BlS+OgFfJM6t@corigine.com>
References: <20230306191824.4115839-1-harshit.m.mogalapalli@oracle.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230306191824.4115839-1-harshit.m.mogalapalli@oracle.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?7Q8+OmhcSVg1huJVT7GBpTKyRMQjZdiz7eJNv0M1umRMACE0THADT8RN2KTP?=
 =?us-ascii?Q?qMG+vYj3tpv1MpMu2kqGauObj+QOxFJumHwFL+sLFTWSwAQKzMN5bjWW+lRt?=
 =?us-ascii?Q?uS2l2J8qBZOuxiHf0webiVZehKfUsT+yJzZ0U7cMJPDDiGWyrky1ao9rkdFD?=
 =?us-ascii?Q?q9l0L6hXFIF/5nkkjmX/ZR3Jn+7zyBkfU4ngs0jdasaNllFvS4Ngg1Xbv0GU?=
 =?us-ascii?Q?cmE2TbzbrofART8UfEQ87/de148GCZcANoU6x9Me3oOIrgMVMpUZ7KSjwvwB?=
 =?us-ascii?Q?GyJleLrMA33uBrbJ188VOHmx8AJJm9l0b5gXM9DbPck1+rzCi1aGMaQzj73I?=
 =?us-ascii?Q?IIKleZgFVTNOrTCj8ZCdXtQXAZe6MKvSPXAzkuzdyRJCSyMdBKQkLMloDEzV?=
 =?us-ascii?Q?hwNI0rns0PHwMj1pp1y3xvbcN19imwkSAyhy0FcRlGyG6SEDgln7/WJxdZjS?=
 =?us-ascii?Q?8+b2qKM7QaoX665rMmwhglgeeGyf1VgOm6jkiP4eTy8LnbcAptl59vQHazSH?=
 =?us-ascii?Q?ELgTUC+b+MKoFs3cVTqrBMRTKIW3fiXGyWw/nixTsMHZ76+cveDqC10BRmwK?=
 =?us-ascii?Q?djviibScY2i6y/9J/R7FczAZWgpM5vPhTHe2xUfVip5D7yoUU7s0C5zhY1RL?=
 =?us-ascii?Q?IkB6dEOrzzGxbjxyezweCvhXnuqBQMpjBM/g6sYWDFWkuJTTEf/NH+VXqJVE?=
 =?us-ascii?Q?itMP88Zo30a201rfF7IqWxlmntG9diortu+Qsqb80C3nLy/T6Ffsq90rQUFV?=
 =?us-ascii?Q?15lC+jGaSm0agRCE3ougWSnnSH74fzuD/GOwFHzCcGk1cXF5VlQhv2Bj1slG?=
 =?us-ascii?Q?3P3oC7g41L7j1ye3IEYygGqwlVgQId9Etr7okxvGixxglovEjtOzEBNyCE0c?=
 =?us-ascii?Q?muwg6tKxenXe8L7tLA5aq4gVB1hQ217NMkL/RZt6XIDkI9bOP+EJ0QIb2iRK?=
 =?us-ascii?Q?cZ4ZtmunoE4RvBPjjuzKflmb/IpaJcCXpxXoZ/6cfuMSJVkvumXYvoTtyq4j?=
 =?us-ascii?Q?4u5q6XstFRQZ0b4oEustf7PCLzNMEu08TDel6irgC0g9yDRY3I6dLdZ1uKnI?=
 =?us-ascii?Q?W37i90HNudMGiYfr75+35uM/vdQWb71yj4fsXQm3bEnrXAm8Mi0elqsYhV7P?=
 =?us-ascii?Q?i7XqqtJ3c9LoNqTiG/QtFGUwDcacMUdtMi6W2LxOwl6mRD7nejKABIgNYVXx?=
 =?us-ascii?Q?J9u7wM/v7I0gQ94Xe716aGpWEOmOh4sbsIr8V2VjRJhRfpAeq2J9UIJeYHlm?=
 =?us-ascii?Q?fLIodWGxxesUrrZdtdqZBBnht3ihlN2xHkLq0NZk4cJz20aXfHL4DRTrkIDE?=
 =?us-ascii?Q?lCpfAxJp+xqm8ZNC2zM+b9OSUZ6pxePqhbXi3+UzNGfymQsEhBZLqkxXz56R?=
 =?us-ascii?Q?9i7kNBXq/NEUyMaGqTsOjzB1cTr1GOtG8VVNAQX52ZuuMsL6mets/qZz+qM1?=
 =?us-ascii?Q?caOj97mb0FIVairSGJlvdG6qiD89+9REBGo+W2TAKiBnTUXD/trKs43n3I2I?=
 =?us-ascii?Q?CVmTSFxuUdE3Foqlksz0Cu4B/7aGbyaLJ74toAFofv44MkbyRvw0Gm8BA59q?=
 =?us-ascii?Q?fRlH7B5vBlX9iSUWcWkJyGz1heLNV6WabIBHaXw91eMALRn7slHTO1S49ttA?=
 =?us-ascii?Q?uIghGbpxrBzopgxnBySYCPjtrZH5SbEiNP+1+Uh3oYeweDnUEE9rcXEFJZYJ?=
 =?us-ascii?Q?8AoWUQ=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9201237c-617b-4353-c369-08db1ee7f157
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Mar 2023 08:42:54.0583
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: orS5+sxn4tDP2q956yaz8PEVlEfjhAkvo+dhThb29elh5vhEfoE4txIf/OBJiYTLMryN1uN7vIsvPqcHKnsdbV8IfyOR8GKegpdt1EPNwnA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR13MB5361
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Mar 06, 2023 at 11:18:24AM -0800, Harshit Mogalapalli wrote:
> mac_len is of type unsigned, which can never be less than zero.
> 
> 	mac_len = ieee802154_hdr_peek_addrs(skb, &header);
> 	if (mac_len < 0)
> 		return mac_len;
> 
> Change this to type int as ieee802154_hdr_peek_addrs() can return negative
> integers, this is found by static analysis with smatch.
> 
> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>

I discussed this briefly with Harshit offline.

The commit referenced above tag does add the call to
ieee802154_hdr_peek_addrs(), an there is a sign miss match between
the return value and the variable.

The code to check the mac_len was added more recently, by the following
commit. However the fixes tag is probably fine as-is, because it's fixing
error handling of a call made in that commit.

6c993779ea1d ("ca8210: fix mac_len negative array access")

Reviewed-by: Simon Horman <simon.horman@corigine.com>