| Message ID | 20221108091357.115738-1-yuancan@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | vhost/vsock: Fix error handling in vhost_vsock_init() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Tue, Nov 08, 2022 at 09:13:57AM +0000, Yuan Can wrote: >A problem about modprobe vhost_vsock failed is triggered with the >following log given: > >modprobe: ERROR: could not insert 'vhost_vsock': Device or resource busy > >The reason is that vhost_vsock_init() returns misc_register() directly >without checking its return value, if misc_register() failed, it returns >without calling vsock_core_unregister() on vhost_transport, resulting the >vhost_vsock can never be installed later. >A simple call graph is shown as below: > > vhost_vsock_init() > vsock_core_register() # register vhost_transport > misc_register() > device_create_with_groups() > device_create_groups_vargs() > dev = kzalloc(...) # OOM happened > # return without unregister vhost_transport > >Fix by calling vsock_core_unregister() when misc_register() returns error. Thanks for this fix! > >Fixes: c0cfa2d8a788 ("vsock: add multi-transports support") Is this the right tag? It seems to me that since the introduction of vhost-vsock we have the same problem (to be solved differently, because with the introduction of multi-transport we refactored the initialization functions). So should we use 433fc58e6bf2 ("VSOCK: Introduce vhost_vsock.ko")? Thanks, Stefano >Signed-off-by: Yuan Can <yuancan@huawei.com> >--- > drivers/vhost/vsock.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > >diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c >index 5703775af129..10a7d23731fe 100644 >--- a/drivers/vhost/vsock.c >+++ b/drivers/vhost/vsock.c >@@ -959,7 +959,14 @@ static int __init vhost_vsock_init(void) > VSOCK_TRANSPORT_F_H2G); > if (ret < 0) > return ret; >- return misc_register(&vhost_vsock_misc); >+ >+ ret = misc_register(&vhost_vsock_misc); >+ if (ret) { >+ vsock_core_unregister(&vhost_transport.transport); >+ return ret; >+ } >+ >+ return 0; > }; > > static void __exit vhost_vsock_exit(void) >-- >2.17.1 >
在 2022/11/8 17:33, Stefano Garzarella 写道: > On Tue, Nov 08, 2022 at 09:13:57AM +0000, Yuan Can wrote: >> A problem about modprobe vhost_vsock failed is triggered with the >> following log given: >> >> modprobe: ERROR: could not insert 'vhost_vsock': Device or resource busy >> >> The reason is that vhost_vsock_init() returns misc_register() directly >> without checking its return value, if misc_register() failed, it returns >> without calling vsock_core_unregister() on vhost_transport, resulting >> the >> vhost_vsock can never be installed later. >> A simple call graph is shown as below: >> >> vhost_vsock_init() >> vsock_core_register() # register vhost_transport >> misc_register() >> device_create_with_groups() >> device_create_groups_vargs() >> dev = kzalloc(...) # OOM happened >> # return without unregister vhost_transport >> >> Fix by calling vsock_core_unregister() when misc_register() returns >> error. > > Thanks for this fix! > >> >> Fixes: c0cfa2d8a788 ("vsock: add multi-transports support") > > Is this the right tag? > > It seems to me that since the introduction of vhost-vsock we have the > same problem (to be solved differently, because with the introduction > of multi-transport we refactored the initialization functions). > > So should we use 433fc58e6bf2 ("VSOCK: Introduce vhost_vsock.ko")? Thanks for pointing out this problem! I will use the correct tag in the v2 patch.
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c index 5703775af129..10a7d23731fe 100644 --- a/drivers/vhost/vsock.c +++ b/drivers/vhost/vsock.c @@ -959,7 +959,14 @@ static int __init vhost_vsock_init(void) VSOCK_TRANSPORT_F_H2G); if (ret < 0) return ret; - return misc_register(&vhost_vsock_misc); + + ret = misc_register(&vhost_vsock_misc); + if (ret) { + vsock_core_unregister(&vhost_transport.transport); + return ret; + } + + return 0; }; static void __exit vhost_vsock_exit(void)
A problem about modprobe vhost_vsock failed is triggered with the following log given: modprobe: ERROR: could not insert 'vhost_vsock': Device or resource busy The reason is that vhost_vsock_init() returns misc_register() directly without checking its return value, if misc_register() failed, it returns without calling vsock_core_unregister() on vhost_transport, resulting the vhost_vsock can never be installed later. A simple call graph is shown as below: vhost_vsock_init() vsock_core_register() # register vhost_transport misc_register() device_create_with_groups() device_create_groups_vargs() dev = kzalloc(...) # OOM happened # return without unregister vhost_transport Fix by calling vsock_core_unregister() when misc_register() returns error. Fixes: c0cfa2d8a788 ("vsock: add multi-transports support") Signed-off-by: Yuan Can <yuancan@huawei.com> --- drivers/vhost/vsock.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)