Skip to content

Commit 983cc2c

Browse files
d-k-cgregkh
authored andcommitted
greybus: raw: fix use-after-free on cdev close
This addresses a use-after-free bug when a raw bundle is disconnected but its chardev is still opened by an application. When the application releases the cdev, it causes the following panic when init on free is enabled (CONFIG_INIT_ON_FREE_DEFAULT_ON=y): refcount_t: underflow; use-after-free. WARNING: CPU: 0 PID: 139 at lib/refcount.c:28 refcount_warn_saturate+0xd0/0x130 ... Call Trace: <TASK> cdev_put+0x18/0x30 __fput+0x255/0x2a0 __x64_sys_close+0x3d/0x80 do_syscall_64+0xa4/0x290 entry_SYSCALL_64_after_hwframe+0x77/0x7f The cdev is contained in the "gb_raw" structure, which is freed in the disconnect operation. When the cdev is released at a later time, cdev_put gets an address that points to freed memory. To fix this use-after-free, convert the struct device from a pointer to being embedded, that makes the lifetime of the cdev and of this device the same. Then, use cdev_device_add, which guarantees that the device won't be released until all references to the cdev have been released. Finally, delegate the freeing of the structure to the device release function, instead of freeing immediately in the disconnect callback. Fixes: e806c7f ("greybus: raw: add raw greybus kernel driver") Reviewed-by: Johan Hovold <johan@kernel.org> Signed-off-by: Damien Riégel <damien.riegel@silabs.com> Link: https://patch.msgid.link/20260324140039.40001-1-damien.riegel@silabs.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent d6696ce commit 983cc2c

1 file changed

Lines changed: 34 additions & 35 deletions

File tree

  • drivers/staging/greybus

drivers/staging/greybus/raw.c

Lines changed: 34 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,8 @@ struct gb_raw {
2121
struct list_head list;
2222
int list_data;
2323
struct mutex list_lock;
24-
dev_t dev;
2524
struct cdev cdev;
26-
struct device *device;
25+
struct device dev;
2726
};
2827

2928
struct raw_data {
@@ -148,6 +147,15 @@ static int gb_raw_send(struct gb_raw *raw, u32 len, const char __user *data)
148147
return retval;
149148
}
150149

150+
static void raw_dev_release(struct device *dev)
151+
{
152+
struct gb_raw *raw = container_of(dev, struct gb_raw, dev);
153+
154+
ida_free(&minors, MINOR(raw->dev.devt));
155+
156+
kfree(raw);
157+
}
158+
151159
static int gb_raw_probe(struct gb_bundle *bundle,
152160
const struct greybus_bundle_id *id)
153161
{
@@ -164,15 +172,30 @@ static int gb_raw_probe(struct gb_bundle *bundle,
164172
if (cport_desc->protocol_id != GREYBUS_PROTOCOL_RAW)
165173
return -ENODEV;
166174

167-
raw = kzalloc_obj(*raw);
168-
if (!raw)
175+
minor = ida_alloc(&minors, GFP_KERNEL);
176+
if (minor < 0)
177+
return minor;
178+
179+
raw = kzalloc_obj(*raw, GFP_KERNEL);
180+
if (!raw) {
181+
ida_free(&minors, minor);
169182
return -ENOMEM;
183+
}
184+
185+
device_initialize(&raw->dev);
186+
raw->dev.devt = MKDEV(raw_major, minor);
187+
raw->dev.class = &raw_class;
188+
raw->dev.parent = &bundle->dev;
189+
raw->dev.release = raw_dev_release;
190+
retval = dev_set_name(&raw->dev, "gb!raw%d", minor);
191+
if (retval)
192+
goto error_put_device;
170193

171194
connection = gb_connection_create(bundle, le16_to_cpu(cport_desc->id),
172195
gb_raw_request_handler);
173196
if (IS_ERR(connection)) {
174197
retval = PTR_ERR(connection);
175-
goto error_free;
198+
goto error_put_device;
176199
}
177200

178201
INIT_LIST_HEAD(&raw->list);
@@ -181,46 +204,26 @@ static int gb_raw_probe(struct gb_bundle *bundle,
181204
raw->connection = connection;
182205
greybus_set_drvdata(bundle, raw);
183206

184-
minor = ida_alloc(&minors, GFP_KERNEL);
185-
if (minor < 0) {
186-
retval = minor;
187-
goto error_connection_destroy;
188-
}
189-
190-
raw->dev = MKDEV(raw_major, minor);
191207
cdev_init(&raw->cdev, &raw_fops);
192208

193209
retval = gb_connection_enable(connection);
194210
if (retval)
195-
goto error_remove_ida;
211+
goto error_connection_destroy;
196212

197-
retval = cdev_add(&raw->cdev, raw->dev, 1);
213+
retval = cdev_device_add(&raw->cdev, &raw->dev);
198214
if (retval)
199215
goto error_connection_disable;
200216

201-
raw->device = device_create(&raw_class, &connection->bundle->dev,
202-
raw->dev, raw, "gb!raw%d", minor);
203-
if (IS_ERR(raw->device)) {
204-
retval = PTR_ERR(raw->device);
205-
goto error_del_cdev;
206-
}
207-
208217
return 0;
209218

210-
error_del_cdev:
211-
cdev_del(&raw->cdev);
212-
213219
error_connection_disable:
214220
gb_connection_disable(connection);
215221

216-
error_remove_ida:
217-
ida_free(&minors, minor);
218-
219222
error_connection_destroy:
220223
gb_connection_destroy(connection);
221224

222-
error_free:
223-
kfree(raw);
225+
error_put_device:
226+
put_device(&raw->dev);
224227
return retval;
225228
}
226229

@@ -231,11 +234,8 @@ static void gb_raw_disconnect(struct gb_bundle *bundle)
231234
struct raw_data *raw_data;
232235
struct raw_data *temp;
233236

234-
// FIXME - handle removing a connection when the char device node is open.
235-
device_destroy(&raw_class, raw->dev);
236-
cdev_del(&raw->cdev);
237+
cdev_device_del(&raw->cdev, &raw->dev);
237238
gb_connection_disable(connection);
238-
ida_free(&minors, MINOR(raw->dev));
239239
gb_connection_destroy(connection);
240240

241241
mutex_lock(&raw->list_lock);
@@ -244,8 +244,7 @@ static void gb_raw_disconnect(struct gb_bundle *bundle)
244244
kfree(raw_data);
245245
}
246246
mutex_unlock(&raw->list_lock);
247-
248-
kfree(raw);
247+
put_device(&raw->dev);
249248
}
250249

251250
/*

0 commit comments

Comments
 (0)