Message-ID: <b285cdee-b591-5266-5a74-e8c08b034f76@intel.com>
Date:   Thu, 2 Feb 2023 09:23:09 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH v3] i40e: Add checking for null for nlmsg_find_attr()
Content-Language: en-US
To:     Natalia Petrova <n.petrova@fintech.ru>,
        Jesse Brandeburg <jesse.brandeburg@intel.com>
CC:     "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        <intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <lvc-project@linuxtesting.org>
References: <20230201090610.52782-1-n.petrova@fintech.ru>
From:   Tony Nguyen <anthony.l.nguyen@intel.com>
In-Reply-To: <20230201090610.52782-1-n.petrova@fintech.ru>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cEU4OE9aZ2pQanZ1NWZpTWFCdjF0OU56eFIyTVVYdXBHMHNqNzljZkpaWTZT?=
 =?utf-8?B?ZXNGWUo5bHpKc2JVNVE3VHVRM2ZYRW1YTFRpKy9CazRRSVNoOUgzbDJMbXln?=
 =?utf-8?B?ak9kYkpBYU95cHZtZy8zaHdqci9PeFFLcWpXUEttaEZNcm9wbC9oZjJENHEz?=
 =?utf-8?B?MWlTVU4zQSs3azljTzNwWm5wdERJNXJRZ0htbHF2UnNyaHpHalZTTHFCUTMw?=
 =?utf-8?B?VWJHT21pZHlweUdadC82KzR6K2lHU2J6b2pteUJVc1JDNXJEM3J1YWVpV09v?=
 =?utf-8?B?WjBFU2NWK0FLbFAwQXVwVXhvL3pkVzJPemVQdlozSkNxdkdSVW4vb1o0aU9S?=
 =?utf-8?B?aURmcm40WjhOby9mYm5MU25wMnlnSHZsalM4MCs0d0lkWDU3aUN5ZHpBb2FD?=
 =?utf-8?B?SWRoTlNBNDd3S0hLbHZZRm04dlhuWnp1MjVFWGVnRW1Ba3ZqKzFYYjRkU0R1?=
 =?utf-8?B?UzlmMnJkeWdvY3g2cmxMcVRWdnlibGFTM1l3blJUWEJ2VmpEK1cyZE5pK2ZW?=
 =?utf-8?B?MFJ4ajZ5ZFFlTGorUERNS1hURm5DUFRLNVFwL1cxbzQ0Y1l5MG5KQmZUY1NC?=
 =?utf-8?B?MHBDWEtEVzg5YTFrajNJL2tZRHdvMWo3SDg3NzVNTllCTHB5SG4rcEdlWGdD?=
 =?utf-8?B?Qms0NFFYdmpITEpVaGMrRjhvRTZvekJ2dUU3bHZCVE1icHRHbkYvcllrbnpN?=
 =?utf-8?B?Z0M5bzBFVjltcVFGQ3hWSFAvd0NlandsbDJvY1JVbUZmNWRqbHF6M2ttNUpZ?=
 =?utf-8?B?SnBRTnZpNUFtRUF1S3gvbVU5YThZSHEzanB1cFdLYzM0N0FZMENNSVp2K00x?=
 =?utf-8?B?aTVLZUE5S21QWmdLTlhjQ1c0VDNxaWg1eTlNdHpQU0krRFJjeDVqay8xQWtr?=
 =?utf-8?B?c1pwZEFjRzJ3SDRpTVlVVm11dGlaM1MrTjFDU3cxTEIvZmYzNHNraEUwVGVG?=
 =?utf-8?B?Z1VYYm9ZaEQ2OGJVek5HcDB5VXJlUEVtZ1JjTWl1eXZTRVNmSEwwb2FUVFV4?=
 =?utf-8?B?KzNMMmI3QkRvV29ic0xyaHoyeno5VWZsR1JxSXpiVEVZUG5DQVI4aUxFbkE3?=
 =?utf-8?B?dWdZOTBhZ0dsb2E1dkdJclNYcHF2c1dEOEp3SlR3RlJSdFZhMUN1RFNlenZ1?=
 =?utf-8?B?M1RBaEdvS2dPMXhPLzF5eWFwU0tPa0o2aTZyRjRQdkIrSHBlUEg3aDJlblNi?=
 =?utf-8?B?aUtZeXpHMTRjVnk4S2Z2emkwVXNiYlF3WXUvQjQyYStMbFR6YUhnM2NNYmF0?=
 =?utf-8?B?YitSa3R4VSs2QUZUbjZPMEFWNUtobXhDalFvUXRXRzNndFlpeVpuUVVvTDNh?=
 =?utf-8?B?NFJhVy9neFMrdWRBYnp4Y0R2eGZEOFdqeEJ0Q0NXb2VKVDFMamFZak5kTWxk?=
 =?utf-8?B?ZWM0R3F6TW1xY1NkRnR5b3IvL05sdjNFNDgxMzZRdlNSbGJjM2F5YTB4ZmlM?=
 =?utf-8?B?NHVJVEdHdG8rRlpkN0M1NUNQKzd2cXFSdml2RnVzbjlneVBDb1hVVVVPZUw3?=
 =?utf-8?B?cGtZdXRiMjRkT0FWUWZGOHVYWWNOLzMvejc3WWl1cFBLWmhTWDZFWnU4eFV2?=
 =?utf-8?B?OHRDS1RvZ2NKMTZUV09JS3NaR1VvcDRhZStWK1orQWE2b2VwVXZrZjhCazlW?=
 =?utf-8?B?Kzh1RzQ1TE5tNFVxZkNqZm1nTHFxYVhVRjRMeWdSeHRGQ1BvZnFrT3d3bm14?=
 =?utf-8?B?NmZ5ZTNJWTU1elU5bjNHMGxPYzN5TDVQYXVGdGJlTlNuZnVrb2pIYWdqcjlu?=
 =?utf-8?B?R2REWnZPT3cwTDNEUy83RmlTOGQxNFlzeFg2alFWT3QwbFR6MDFIeno2VUVv?=
 =?utf-8?B?UU14TWdsOHFWM0ZON080Z01IMFBNQkNDeE51YTI5L0JEaWdsSUNoNUY1Wk80?=
 =?utf-8?B?SHhxL0IvQXBCcFpTS2xDZnVHS2lhUEovS1J1R0lvd0lRdStQeVIwUzV2dDJN?=
 =?utf-8?B?dUk4U3ViTTRWVEZBTHNyYzhENlBpczVDMERycFJkenQwMWI0MU0vT0QzSW1i?=
 =?utf-8?B?eEZDZTVTR1pYUEFuVXVZWHgrY2JsKzN3RjcrakZnNndrMjFSWlRneUMyd0tU?=
 =?utf-8?B?M1ZUMk55VUJjTDNKdVFYUG10M3FxdFdlMU1nMXdQOTJVOEhRR0hmc2txOU9k?=
 =?utf-8?B?S3JLYUUvYlVqWjNWU2twNklQa29nQ3g5UWZwdWNBRTBObVBGYVphbFlWMWdm?=
 =?utf-8?B?MlE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 79dc470f-f35a-47b7-4fd0-08db054229b5
X-MS-Exchange-CrossTenant-AuthSource: SN6PR11MB3229.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Feb 2023 17:23:13.0504
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: gojWtP9FlROxXG1PByInrMoVlErlZ18XhHu6SOU4Z6iE2CATnz2apLcd6I+530hXLmWXeJgwKPKljYgHEm/I2cXFtqRgaG9BoNpYITmrDDY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5926
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2/1/2023 1:06 AM, Natalia Petrova wrote:
> The result of nlmsg_find_attr() 'br_spec' is dereferenced in
> nla_for_each_nested(), but it can take NULL value in nla_find() function,
> which will result in an error.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 51616018dd1b ("i40e: Add support for getlink, setlink ndo ops")
> Signed-off-by: Natalia Petrova <n.petrova@fintech.ru>
> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---

Thanks for the patch. I've applied it, however, for the future if you 
could specify the target tree. Simon mentioned it in v2, but his example 
omitted the tree.

i.e.
[PATCH v3 net] i40e: Add checking for null for nlmsg_find_attr()

Thanks,
Tony

> v3: Fixed mailing list.
> v2: The remark about the error code by Simon Horman <simon.horman@corigine.com>
> was taken into account; return value -ENOENT was changed to -EINVAL.
>   drivers/net/ethernet/intel/i40e/i40e_main.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 53d0083e35da..4626d2a1af91 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -13167,6 +13167,8 @@ static int i40e_ndo_bridge_setlink(struct net_device *dev,
>   	}
>   
>   	br_spec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
> +	if (!br_spec)
> +		return -EINVAL;
>   
>   	nla_for_each_nested(attr, br_spec, rem) {
>   		__u16 mode;