Messages in this thread | | | Date | Mon, 7 Nov 2022 10:06:48 +0200 | From | Leon Romanovsky <> | Subject | Re: [PATCH net] xfrm: Fix ignored return value in xfrm6_init() |
| |
On Mon, Nov 07, 2022 at 11:22:40AM +0800, Chen Zhongjin wrote: > Hi, > > On 2022/11/7 3:08, Leon Romanovsky wrote: > > On Thu, Nov 03, 2022 at 05:07:13PM +0800, Chen Zhongjin wrote: > > > When IPv6 module initializing in xfrm6_init(), register_pernet_subsys() > > > is possible to fail but its return value is ignored. > > > > > > If IPv6 initialization fails later and xfrm6_fini() is called, > > > removing uninitialized list in xfrm6_net_ops will cause null-ptr-deref: > > > > > > KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f] > > > CPU: 1 PID: 330 Comm: insmod > > > RIP: 0010:unregister_pernet_operations+0xc9/0x450 > > > Call Trace: > > > <TASK> > > > unregister_pernet_subsys+0x31/0x3e > > > xfrm6_fini+0x16/0x30 [ipv6] > > > ip6_route_init+0xcd/0x128 [ipv6] > > > inet6_init+0x29c/0x602 [ipv6] > > > ... > > > > > > Fix it by catching the error return value of register_pernet_subsys(). > > > > > > Fixes: 8d068875caca ("xfrm: make gc_thresh configurable in all namespaces") > > > Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com> > > > --- > > > net/ipv6/xfrm6_policy.c | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > I see same error in net/ipv4/xfrm4_policy.c which introduced by same > > commit mentioned in Fixes line. > > It's true that in xfrm4_init() the ops->init is possible to fail as well. > > However there is no error handling or exit path for ipv4, so IIUC the ops > won't be unregistered anyway. > > Considering that ipv4 don't handle most of error in initialization, maybe > it's better to keep it as it is?
Yeah, makes sense.
Thanks, Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
| |