* [PATCH v2 0/2] Drivers: hv: vmbus: Fix the error paths of channel setup
@ 2026-11-03 8:03 Yuqiang Sun
2026-11-03 8:03 ` [PATCH v2 1/2] Drivers: hv: vmbus: Keep the channel when vmbus_add_channel_kobj() fails Yuqiang Sun
2026-11-03 8:03 ` [PATCH v2 2/2] Drivers: hv: vmbus: Do not remove the channel when device registration fails Yuqiang Sun
0 siblings, 2 replies; 4+ messages in thread
From: Yuqiang Sun @ 2026-11-03 8:03 UTC (permalink / raw)
To: haiyangz, wei.liu, decui
Cc: mhklinux, stephen, linmq006, linux-hyperv, linux-kernel
When setting up a channel fails, vmbus_add_channel_work() and
vmbus_device_register() do not agree on who cleans up:
- vmbus_add_channel_kobj() drops the channel's last reference when it
fails, and its callers then use the channel and drop it again (patch 1;
this is what our static analysis reported, for the sub-channel path).
Paths that fail before it put a kobject that was never initialized.
- When vmbus_device_register() fails, the device's release calls
hv_process_channel_removal() on a channel that was not rescinded, which
hits BUG_ON(!channel->rescind); vmbus_add_channel_work() would then
clean the channel up a second time (patch 2). Patch 2 leaves the
channel to vmbus_add_channel_work() when device_register() fails, and
keeps the device when only the channel's sysfs entries fail, since a
driver may be bound by then.
Changes in v2 (patch 2 only, after the Sashiko review of v1):
- vmbus_match() skips a device whose channel pointer was cleared, which
a concurrent driver_attach() can still see.
- Failures after device_register() (the channels directory, the primary
channel's kobject and its sysfs group) no longer unregister the
device: a bound driver such as hv_sock, or one that opened
sub-channels, can keep the channel past its remove callback unless the
channel was rescinded. A sub-channel of a device without a channels
directory is refused.
v1: https://lore.kernel.org/linux-hyperv/20261102094719.35760-1-izaiahsun@gmail.com/
Testing: QEMU (x86_64, KVM, kvm=off with the hv-* enlightenments,
vmbus-bridge and hv-balloon) with KASAN, hv_vmbus and hv_balloon as
modules loaded from init after failslab is armed, on mainline
d135ab453a74. failslab is limited to allocations under
vmbus_device_register(), and fails the N-th one when the hv-balloon
offer is processed. Without the series, every failure hits the BUG_ON.
With it, for N = 1 to 95:
- 53 failures in device_register() leave the offer unbound with
"unable to add child device object";
- 31 failures in the channels directory, the channel's kobject or its
sysfs group leave the device registered and bound to hv_balloon, with
the channel missing from sysfs; unbinding the driver and binding it
again works;
- the other runs fail an allocation whose failure is handled, or none.
No run reports anything (no BUG, KASAN, WARNING or refcount_t message).
The sub-channel paths are not covered: QEMU's VMBus offers no
sub-channels, cannot rescind a channel, and hv_balloon cannot be
unloaded.
The failures need an allocation failure while the guest processes a
channel offer from the host.
Yuqiang Sun (2):
Drivers: hv: vmbus: Keep the channel when vmbus_add_channel_kobj()
fails
Drivers: hv: vmbus: Do not remove the channel when device registration
fails
drivers/hv/channel_mgmt.c | 5 +--
drivers/hv/hyperv_vmbus.h | 1 +
drivers/hv/vmbus_drv.c | 71 +++++++++++++++++++++++----------------
3 files changed, 46 insertions(+), 31 deletions(-)
base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
--
2.53.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/2] Drivers: hv: vmbus: Keep the channel when vmbus_add_channel_kobj() fails
2026-11-03 8:03 [PATCH v2 0/2] Drivers: hv: vmbus: Fix the error paths of channel setup Yuqiang Sun
@ 2026-11-03 8:03 ` Yuqiang Sun
2026-11-03 8:03 ` [PATCH v2 2/2] Drivers: hv: vmbus: Do not remove the channel when device registration fails Yuqiang Sun
1 sibling, 0 replies; 4+ messages in thread
From: Yuqiang Sun @ 2026-11-03 8:03 UTC (permalink / raw)
To: haiyangz, wei.liu, decui
Cc: mhklinux, stephen, linmq006, linux-hyperv, linux-kernel, stable
The channel is freed by the release of its kobject, and free_channel()
drops that reference. Since commit 8bc69f86328e ("Drivers: hv: vmbus:
Fix memory leak in vmbus_add_channel_kobj"), vmbus_add_channel_kobj()
also drops it when it fails, as kobject_init_and_add() asks for. That
frees the channel, and both callers then go on with it:
vmbus_add_channel_work() unlinks a sub-channel, unmaps and releases its
relid and calls free_channel(), a use after free and a second
kobject_put(); for a primary channel, vmbus_device_register() unwinds
through vmbus_device_release(), which removes the channel too.
The kobject is also only initialized in vmbus_add_channel_kobj(), so the
error paths before it, for instance a vmbus_device_create() failure in
vmbus_add_channel_work(), call free_channel() on a kobject that was
never initialized: kobject_put() warns and does nothing, so the channel
is leaked, and removing the attribute group from a kobject that is not
in sysfs warns as well.
Initialize the kobject when the channel is allocated, so that
free_channel() is the one place that drops its reference on every path.
vmbus_add_channel_kobj() then only adds it, and leaves the reference to
its callers when it fails. Remove the attribute group only if the
kobject was added.
Fixes: c2e5df616e1a ("vmbus: add per-channel sysfs info")
Fixes: 8bc69f86328e ("Drivers: hv: vmbus: Fix memory leak in vmbus_add_channel_kobj")
Cc: stable@vger.kernel.org
Signed-off-by: Yuqiang Sun <izaiahsun@gmail.com>
---
Notes:
The bug was found with kbound, a research static analysis prototype.
The patch, the changelog and the test setup were written with the help
of an AI coding assistant (Claude); I have reviewed them and tested them
as described in the cover letter.
drivers/hv/channel_mgmt.c | 5 +++--
drivers/hv/hyperv_vmbus.h | 1 +
drivers/hv/vmbus_drv.c | 25 ++++++++++++++++++-------
3 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index c42def04ac41..cb1d8d76928f 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -367,6 +367,7 @@ static struct vmbus_channel *alloc_channel(void)
vmbus_on_event, (unsigned long)channel);
hv_ringbuffer_pre_init(channel);
+ vmbus_init_channel_kobj(channel);
return channel;
}
@@ -663,8 +664,8 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
mutex_unlock(&vmbus_connection.channel_mutex);
cpus_read_unlock();
/*
- * Don't call free_channel(), because newchannel->kobj
- * is not initialized yet.
+ * Don't call free_channel(): newchannel was never
+ * added, and its kobject holds nothing yet.
*/
kfree(newchannel);
WARN_ON_ONCE(1);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 33923621a5a3..7865a519dd7f 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -408,6 +408,7 @@ struct hv_device *vmbus_device_create(const guid_t *type,
int vmbus_device_register(struct hv_device *child_device_obj);
void vmbus_device_unregister(struct hv_device *device_obj);
+void vmbus_init_channel_kobj(struct vmbus_channel *channel);
int vmbus_add_channel_kobj(struct hv_device *device_obj,
struct vmbus_channel *channel);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 4692687cdbf3..fc0f16f51266 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2084,6 +2084,15 @@ int hv_remove_ring_sysfs(struct vmbus_channel *channel)
}
EXPORT_SYMBOL_GPL(hv_remove_ring_sysfs);
+/*
+ * vmbus_init_channel_kobj - initialize the kobject that holds the channel's
+ * lifetime; free_channel() drops its reference
+ */
+void vmbus_init_channel_kobj(struct vmbus_channel *channel)
+{
+ kobject_init(&channel->kobj, &vmbus_chan_ktype);
+}
+
/*
* vmbus_add_channel_kobj - setup a sub-directory under device/channels
*/
@@ -2094,13 +2103,14 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
u32 relid = channel->offermsg.child_relid;
int ret;
+ /*
+ * On failure the kobject keeps its reference: the callers' error
+ * paths drop it in free_channel(), which also frees the channel.
+ */
kobj->kset = dev->channels_kset;
- ret = kobject_init_and_add(kobj, &vmbus_chan_ktype, NULL,
- "%u", relid);
- if (ret) {
- kobject_put(kobj);
+ ret = kobject_add(kobj, NULL, "%u", relid);
+ if (ret)
return ret;
- }
ret = sysfs_create_group(kobj, &vmbus_chan_group);
@@ -2109,7 +2119,6 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
* The calling functions' error handling paths will cleanup the
* empty channel directory.
*/
- kobject_put(kobj);
dev_err(device, "Unable to set up channel sysfs files\n");
return ret;
}
@@ -2124,7 +2133,9 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
*/
void vmbus_remove_channel_attr_group(struct vmbus_channel *channel)
{
- sysfs_remove_group(&channel->kobj, &vmbus_chan_group);
+ /* A channel that failed before vmbus_add_channel_kobj() has none */
+ if (channel->kobj.state_in_sysfs)
+ sysfs_remove_group(&channel->kobj, &vmbus_chan_group);
}
/*
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] Drivers: hv: vmbus: Do not remove the channel when device registration fails
2026-11-03 8:03 [PATCH v2 0/2] Drivers: hv: vmbus: Fix the error paths of channel setup Yuqiang Sun
2026-11-03 8:03 ` [PATCH v2 1/2] Drivers: hv: vmbus: Keep the channel when vmbus_add_channel_kobj() fails Yuqiang Sun
@ 2026-11-03 8:03 ` Yuqiang Sun
2026-11-03 8:37 ` sashiko-bot
1 sibling, 1 reply; 4+ messages in thread
From: Yuqiang Sun @ 2026-11-03 8:03 UTC (permalink / raw)
To: haiyangz, wei.liu, decui
Cc: mhklinux, stephen, linmq006, linux-hyperv, linux-kernel, stable
When vmbus_device_register() fails, it drops the device it was setting
up, and the device's release, vmbus_device_release(), calls
hv_process_channel_removal() on the channel. That function is for
channels the host has rescinded and starts with
BUG_ON(!channel->rescind), so the kernel stops there. Without the
BUG_ON, it would unmap and release the relid, unlink the channel and
free it, and then the error path of vmbus_add_channel_work() would do
all of that again.
When device_register() fails, clear the device's channel pointer before
dropping the device, and only remove the channel in
vmbus_device_release() if there is one: vmbus_add_channel_work() cleans
it up. A concurrent driver_attach() can still match the device while it
is being removed, so make vmbus_match() skip a device without a channel.
The later failures, creating the channels directory or the primary
channel's kobject, come after device_register() may have bound a driver,
and a driver can hold on to the channel past its remove callback unless
the channel was rescinded (hv_sock's connection, or the sub-channels it
opened). So do not unregister the device for them: its channel is then
just missing from sysfs, as it is while the device is being set up. A
sub-channel of such a device is refused, as when its own kobject cannot
be added.
Fixes: c2e5df616e1a ("vmbus: add per-channel sysfs info")
Cc: stable@vger.kernel.org
Signed-off-by: Yuqiang Sun <izaiahsun@gmail.com>
---
Notes:
The bug was found with kbound, a research static analysis prototype.
The patch, the changelog and the test setup were written with the help
of an AI coding assistant (Claude); I have reviewed them and tested them
as described in the cover letter.
Changes in v2: vmbus_match() skips a device without a channel, and the
failures after device_register() keep the device instead of
unregistering it (Sashiko review of v1).
drivers/hv/vmbus_drv.c | 46 ++++++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index fc0f16f51266..1b9bc82bebd0 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -840,9 +840,14 @@ static int vmbus_match(struct device *device, const struct device_driver *driver
{
const struct hv_driver *drv = drv_to_hv_drv(driver);
struct hv_device *hv_dev = device_to_hv_device(device);
+ struct vmbus_channel *channel = READ_ONCE(hv_dev->channel);
+
+ /* None if vmbus_device_register() failed */
+ if (!channel)
+ return 0;
/* The hv_sock driver handles all hv_sock offers. */
- if (is_hvsock_channel(hv_dev->channel))
+ if (is_hvsock_channel(channel))
return drv->hvsock;
if (hv_vmbus_get_id(drv, hv_dev))
@@ -979,9 +984,15 @@ static void vmbus_device_release(struct device *device)
hv_debug_rm_dev_dir(hv_dev);
- mutex_lock(&vmbus_connection.channel_mutex);
- hv_process_channel_removal(channel);
- mutex_unlock(&vmbus_connection.channel_mutex);
+ /*
+ * No channel if vmbus_device_register() failed: the channel was not
+ * rescinded, and vmbus_add_channel_work() cleans it up.
+ */
+ if (channel) {
+ mutex_lock(&vmbus_connection.channel_mutex);
+ hv_process_channel_removal(channel);
+ mutex_unlock(&vmbus_connection.channel_mutex);
+ }
kfree(hv_dev);
}
@@ -2103,6 +2114,10 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
u32 relid = channel->offermsg.child_relid;
int ret;
+ /* The device has no channels directory if creating it failed */
+ if (!dev->channels_kset)
+ return -ENOENT;
+
/*
* On failure the kobject keeps its reference: the callers' error
* paths drop it in free_channel(), which also frees the channel.
@@ -2189,6 +2204,7 @@ int vmbus_device_register(struct hv_device *child_device_obj)
ret = device_register(&child_device_obj->device);
if (ret) {
pr_err("Unable to register child device\n");
+ WRITE_ONCE(child_device_obj->channel, NULL);
put_device(&child_device_obj->device);
return ret;
}
@@ -2205,31 +2221,17 @@ int vmbus_device_register(struct hv_device *child_device_obj)
* here. The race can't result in a kernel failure, but the user space
* program may get an error in accessing "channels" or its
* subdirectories. See also comments with vmbus_add_dynid() about a
- * related race condition.
+ * related race condition. For the same reason, the device stays
+ * registered if its channel cannot be shown in sysfs.
*/
child_device_obj->channels_kset = kset_create_and_add("channels",
NULL, kobj);
- if (!child_device_obj->channels_kset) {
- ret = -ENOMEM;
- goto err_dev_unregister;
- }
-
- ret = vmbus_add_channel_kobj(child_device_obj,
- child_device_obj->channel);
- if (ret) {
+ if (!child_device_obj->channels_kset ||
+ vmbus_add_channel_kobj(child_device_obj, child_device_obj->channel))
pr_err("Unable to register primary channel\n");
- goto err_kset_unregister;
- }
hv_debug_add_dev_dir(child_device_obj);
return 0;
-
-err_kset_unregister:
- kset_unregister(child_device_obj->channels_kset);
-
-err_dev_unregister:
- device_unregister(&child_device_obj->device);
- return ret;
}
/*
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] Drivers: hv: vmbus: Do not remove the channel when device registration fails
2026-11-03 8:03 ` [PATCH v2 2/2] Drivers: hv: vmbus: Do not remove the channel when device registration fails Yuqiang Sun
@ 2026-11-03 8:37 ` sashiko-bot
0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-11-03 8:37 UTC (permalink / raw)
To: Yuqiang Sun; +Cc: linux-hyperv
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] hv: vmbus: dangling channel->device_obj on vmbus_device_register() failure
- [High] hv: vmbus: sub-channel refusal in vmbus_add_channel_kobj() causes deadlock
- [High] hv: vmbus: unlocked channel dereference in vmbus_match() without RCU
- [Medium] hv: vmbus: leaked sysfs state when primary vmbus_add_channel_kobj() fails
--
commit 1c2eec91c8b747f69eceb0c4ce1f5ab28e5429b8
Author: Yuqiang Sun <izaiahsun@gmail.com>
Drivers: hv: vmbus: Do not remove the channel when device registration fails
This commit avoids calling hv_process_channel_removal() in
vmbus_device_release() when device_register() fails by clearing the
device's channel pointer beforehand, and keeps the device registered if
creating the channels kset or primary channel kobject fails.
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index fc0f16f512669..1b9bc82bebd06 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -840,9 +840,14 @@ static int vmbus_match(struct device *device, const struct device_driver *driver
> {
> const struct hv_driver *drv = drv_to_hv_drv(driver);
> struct hv_device *hv_dev = device_to_hv_device(device);
> + struct vmbus_channel *channel = READ_ONCE(hv_dev->channel);
> +
> + /* None if vmbus_device_register() failed */
> + if (!channel)
> + return 0;
>
> /* The hv_sock driver handles all hv_sock offers. */
> - if (is_hvsock_channel(hv_dev->channel))
> + if (is_hvsock_channel(channel))
> return drv->hvsock;
[Severity: High]
As reported by Sashiko on v1 [1], can channel be freed before
is_hvsock_channel(channel) dereferences it here?
During device_register() -> device_add(), bus_add_device() links the
device onto the bus klist_devices before later steps in device_add()
(such as dpm_sysfs_add()) can fail. On failure, bus_remove_device() calls
klist_del(), which does not wait for a concurrent __driver_attach() (for
example via vmbus_add_dynid()) that already took a device reference in
bus_for_each_dev() and runs driver_match_device() -> vmbus_match() before
acquiring device_lock(dev).
If vmbus_match() loads hv_dev->channel with READ_ONCE() just before
vmbus_device_register() clears child_device_obj->channel to NULL, the
registering thread returns to vmbus_add_channel_work(), which runs
err_deq_chan and calls free_channel() -> kobject_put() ->
vmbus_chan_release() -> kfree_rcu(channel, rcu).
Because vmbus_match() runs in preemptible process context without
rcu_read_lock(), if the matching thread is preempted after loading
channel, could the RCU grace period complete and free channel before
is_hvsock_channel(channel) reads channel->offermsg.offer.chn_flags?
[ ... ]
> @@ -2103,6 +2114,10 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
> u32 relid = channel->offermsg.child_relid;
> int ret;
>
> + /* The device has no channels directory if creating it failed */
> + if (!dev->channels_kset)
> + return -ENOENT;
> +
> /*
> * On failure the kobject keeps its reference: the callers' error
> * paths drop it in free_channel(), which also frees the channel.
[Severity: High]
Does returning -ENOENT when !dev->channels_kset cause sub-channel setup
to deadlock or hang system suspend?
In vmbus_device_register(), device_register() runs before
kset_create_and_add("channels", ...) assigns child_device_obj->channels_kset,
and if kset_create_and_add() fails, the device now stays registered and
bound to its driver with channels_kset == NULL.
During device_register() -> device_add() -> bus_probe_device(), drivers
such as hv_storvsc and hv_netvsc request sub-channels from Hyper-V before
device_register() returns. Because vmbus_process_offer() queues
sub-channel work onto handle_sub_chan_wq concurrently with
handle_primary_chan_wq, vmbus_add_channel_work() for an offered
sub-channel can call vmbus_add_channel_kobj() either before
kset_create_and_add() sets dev->channels_kset or after
kset_create_and_add() fails:
drivers/hv/channel_mgmt.c:vmbus_add_channel_work() {
...
if (vmbus_add_channel_kobj(dev, newchannel))
goto err_deq_chan;
if (primary_channel->sc_creation_callback != NULL)
primary_channel->sc_creation_callback(newchannel);
...
}
When vmbus_add_channel_kobj() fails and jumps to err_deq_chan,
sc_creation_callback is skipped. In hv_netvsc, netvsc_subchan_work() ->
rndis_set_subchannel() holds rtnl_lock() and waits uninterruptibly for
netvsc_sc_open() to increment nvdev->open_chn for each requested
sub-channel:
drivers/net/hyperv/rndis_filter.c:rndis_set_subchannel() {
...
/* wait for all sub channels to open */
wait_event(nvdev->subchan_open,
atomic_read(&nvdev->open_chn) == nvdev->num_chn);
}
In addition, vmbus_process_offer() increments
vmbus_connection.nr_chan_close_on_suspend for every sub-channel and
allocates a target CPU in init_vp_index(), but err_deq_chan in
vmbus_add_channel_work() frees newchannel without decrementing
nr_chan_close_on_suspend or calling hv_clear_allocated_cpu(). Wouldn't
this leave rndis_set_subchannel() deadlocked holding rtnl_lock(), and
cause vmbus_bus_suspend() to hang forever in
wait_for_completion(&vmbus_connection.ready_for_suspend_event)?
> @@ -2189,6 +2204,7 @@ int vmbus_device_register(struct hv_device *child_device_obj)
> ret = device_register(&child_device_obj->device);
> if (ret) {
> pr_err("Unable to register child device\n");
> + WRITE_ONCE(child_device_obj->channel, NULL);
> put_device(&child_device_obj->device);
> return ret;
> }
[Severity: Critical]
When device_register() fails here and put_device() frees
child_device_obj via vmbus_device_release(), does newchannel->device_obj
remain a dangling pointer to the freed struct hv_device?
In vmbus_add_channel_work(), newchannel->device_obj is assigned before
calling vmbus_device_register(newchannel->device_obj), and on failure it
jumps to err_deq_chan without clearing newchannel->device_obj:
drivers/hv/channel_mgmt.c:vmbus_add_channel_work() {
...
ret = vmbus_device_register(newchannel->device_obj);
if (ret != 0) {
pr_err("unable to add child device object (relid %d)\n",
newchannel->offermsg.child_relid);
goto err_deq_chan;
}
...
err_deq_chan:
mutex_lock(&vmbus_connection.channel_mutex);
...
WRITE_ONCE(newchannel->probe_done, true);
}
If the host rescinds the channel before err_deq_chan unmaps the relid,
vmbus_onoffer_rescind() looks up the channel via relid2channel(), waits
for channel->probe_done to become true, and then checks
channel->device_obj:
drivers/hv/channel_mgmt.c:vmbus_onoffer_rescind() {
...
if (channel->device_obj) {
if (channel->chn_rescind_callback) {
channel->chn_rescind_callback(channel);
...
return;
}
dev = get_device(&channel->device_obj->device);
if (dev) {
vmbus_device_unregister(channel->device_obj);
put_device(dev);
}
}
}
Once err_deq_chan sets probe_done = true and frees newchannel, wouldn't
vmbus_onoffer_rescind() dereference the already-freed
channel->device_obj (and access channel while err_deq_chan is freeing
it)?
> @@ -2205,31 +2221,17 @@ int vmbus_device_register(struct hv_device *child_device_obj)
> * here. The race can't result in a kernel failure, but the user space
> * program may get an error in accessing "channels" or its
> * subdirectories. See also comments with vmbus_add_dynid() about a
> - * related race condition.
> + * related race condition. For the same reason, the device stays
> + * registered if its channel cannot be shown in sysfs.
> */
> child_device_obj->channels_kset = kset_create_and_add("channels",
> NULL, kobj);
> - if (!child_device_obj->channels_kset) {
> - ret = -ENOMEM;
> - goto err_dev_unregister;
> - }
> -
> - ret = vmbus_add_channel_kobj(child_device_obj,
> - child_device_obj->channel);
> - if (ret) {
> + if (!child_device_obj->channels_kset ||
> + vmbus_add_channel_kobj(child_device_obj, child_device_obj->channel))
> pr_err("Unable to register primary channel\n");
[Severity: Medium]
What happens to child_device_obj->channel->kobj and
child_device_obj->channels_kset if kset_create_and_add() succeeds,
kobject_add() inside vmbus_add_channel_kobj() succeeds, and then
sysfs_create_group() fails?
In vmbus_add_channel_kobj(), if sysfs_create_group() fails, kobj is not
removed from sysfs because the function relies on the caller's error
path to clean up the channel:
drivers/hv/vmbus_drv.c:vmbus_add_channel_kobj() {
...
kobj->kset = dev->channels_kset;
ret = kobject_add(kobj, NULL, "%u", relid);
if (ret)
return ret;
ret = sysfs_create_group(kobj, &vmbus_chan_group);
if (ret) {
/*
* The calling functions' error handling paths will cleanup the
* empty channel directory.
*/
dev_err(device, "Unable to set up channel sysfs files\n");
return ret;
}
...
}
Because vmbus_device_register() no longer unwinds on
vmbus_add_channel_kobj() failure, doesn't this leave an empty
/sys/bus/vmbus/devices/<dev>/channels/<relid> directory in sysfs with
kobj->state_in_sysfs == 1 (causing vmbus_remove_channel_attr_group() to
later call sysfs_remove_group() on a group that failed creation), while
also leaving child_device_obj->channels_kset non-NULL so sub-channels
are not refused as described in the commit message?
[1] https://lore.kernel.org/r/20261102094719.35760-3-izaiahsun@gmail.com
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103080347.90310-1-izaiahsun@gmail.com?part=2
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-03 8:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03 8:03 [PATCH v2 0/2] Drivers: hv: vmbus: Fix the error paths of channel setup Yuqiang Sun
2026-11-03 8:03 ` [PATCH v2 1/2] Drivers: hv: vmbus: Keep the channel when vmbus_add_channel_kobj() fails Yuqiang Sun
2026-11-03 8:03 ` [PATCH v2 2/2] Drivers: hv: vmbus: Do not remove the channel when device registration fails Yuqiang Sun
2026-11-03 8:37 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox