Skip to content

Commit bcba013

Browse files
committed
fix memory corruption by read from removed items
An item unregistered via procstat_remove may still be reached by fuse_read(): the item itself has refcnt held from fuse_open. In such case, the owner may have freed the item stat memory, which is still ok to read. HOWEVER, writing to this memory is prohibited and may cause memory corruption. Write is possible only for series (see is_reset() and clear_values_...) called from e.g. series_u64_read(). The item itself may not be marked as unregistered (refcnt != 0 in item_put_locked), but since series are removed by directory we can rely on parent being marked as unregistered by procstat_remove(). Same applies to reads by aggregator_read(). NOTE: after procstat_remove() releases the mutex, changes to the item (flags) should be observable by fuse_read() and aggregator_read() from other cpus. - fuse_read() does not take the mutex, so this is suspicious. - aggregator_read() does, so the protection is clean. The recommendation is not not read series stats that may be removed (volumes) unless via an aggregator. Issue: LBM1-15406 Signed-off-by: Anton Eidelman <anton@lightbitslabs.com>
1 parent c09dba2 commit bcba013

File tree

1 file changed

+13
-1
lines changed

1 file changed

+13
-1
lines changed

src/procstat.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,10 @@ static int out_item(struct out_stream *out, char *path, struct procstat_item *it
546546
int p_space = MAX_PATH_LEN - path_len;
547547
struct procstat_directory *dir = container_of(item, struct procstat_directory, base);
548548

549+
/* See fuse_read(): it is unsafe to read files under a directory that is marked unregistered */
550+
if (!item_registered(item))
551+
return 0;
552+
549553
if (pos && p_space) {
550554
path[pos++] = '/';
551555
--p_space;
@@ -693,7 +697,15 @@ static void fuse_read(fuse_req_t req, fuse_ino_t ino, size_t size, off_t off, st
693697
return;
694698
}
695699

696-
if (!file->fmt) {
700+
/*
701+
* An item unregistered via procstat_remove may still be reached here: the item itself has refcnt held from fuse_open.
702+
* If so, the owner may have freed the item stat memory, which is still ok to read.
703+
* HOWEVER, writing to this memory is prohibited and may cause memory corruption.
704+
* Write is possible only for series (see is_reset() and clear_values_...).
705+
* The item itself may not be marked as unregistered (refcnt != 0 in item_put_locked),
706+
* but since series are removed by directory we can rely on parent being marked as unregistered by procstat_remove().
707+
*/
708+
if (!file->fmt || !file->base.parent || !item_registered(&file->base.parent->base)) {
697709
fuse_reply_buf(req, NULL, 0);
698710
return;
699711
}

0 commit comments

Comments
 (0)