Re: [PATCH net] net: openvswitch: reject negative ifindex

[Leon Romanovsky <leon@kernel.org>]

On Mon, Aug 14, 2023 at 01:38:40PM -0700, Jakub Kicinski wrote:
> Recent changes in net-next (commit 759ab1edb56c ("net: store netdevs
> in an xarray")) refactored the handling of pre-assigned ifindexes
> and let syzbot surface a latent problem in ovs. ovs does not validate
> ifindex, making it possible to create netdev ports with negative
> ifindex values. It's easy to repro with YNL:
> 
> $ ./cli.py --spec netlink/specs/ovs_datapath.yaml \
>          --do new \
> 	 --json '{"upcall-pid": 1, "name":"my-dp"}'
> $ ./cli.py --spec netlink/specs/ovs_vport.yaml \
> 	 --do new \
> 	 --json '{"upcall-pid": "00000001", "name": "some-port0", "dp-ifindex":3,"ifindex":4294901760,"type":2}'
> 
> $ ip link show
> -65536: some-port0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
>     link/ether 7a:48:21:ad:0b:fb brd ff:ff:ff:ff:ff:ff
> ...
> 
> Validate the inputes. Now the second command correctly returns:
> 
> $ ./cli.py --spec netlink/specs/ovs_vport.yaml \
> 	 --do new \
> 	 --json '{"upcall-pid": "00000001", "name": "some-port0", "dp-ifindex":3,"ifindex":4294901760,"type":2}'
> 
> lib.ynl.NlError: Netlink error: Numerical result out of range
> nl_len = 108 (92) nl_flags = 0x300 nl_type = 2
> 	error: -34	extack: {'msg': 'integer out of range', 'unknown': [[type:4 len:36] b'\x0c\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x0c\x00\x03\x00\xff\xff\xff\x7f\x00\x00\x00\x00\x08\x00\x01\x00\x08\x00\x00\x00'], 'bad-attr': '.ifindex'}
> 
> Accept 0 since it used to be silently ignored.
> 
> Fixes: 54c4ef34c4b6 ("openvswitch: allow specifying ifindex of new interfaces")
> Reported-by: syzbot+7456b5dcf65111553320@syzkaller.appspotmail.com
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: pshelar@ovn.org
> CC: andrey.zhadchenko@virtuozzo.com
> CC: brauner@kernel.org
> CC: dev@openvswitch.org
> ---
>  net/openvswitch/datapath.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>