Bug 263371 - Possible memleak bugs caused by g_raid_destroy_volume()
Summary: Possible memleak bugs caused by g_raid_destroy_volume()
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: Unspecified
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-04-17 16:02 UTC by Zhou Qingyang
Modified: 2022-04-17 17:47 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Zhou Qingyang 2022-04-17 16:02:03 UTC
In sys/geom/raid/g_raid.c file, the function g_raid_destroy_volume(vol) will not 
free "vol" on some paths and return EBUSY instead of 0.

However, not all caller of g_raid_destroy_volume() will check the return value and 
those callers assume that there is no error when running 
g_raid_destroy_volume(vol).

In detail, functions below does not check the return value:

In sys/geom/raid/md_promise.c file,
function: g_raid_md_promise_purge_volumes() and g_raid_md_ctl_promise()

In sys/geom/raid/md_ddf.c file,
fucntion: g_raid_md_ddf_purge_volumes() and g_raid_md_ctl_ddf()

In sys/geom/raid/md_intel.c file,
function: g_raid_md_ctl_intel()

In sys/geom/raid/g_raid.c file,
function: g_raid_update_volume() and g_raid_access().

There only one function g_raid_destroy_node() checks the return value of 
g_raid_destroy_volume() and return EBUSY.
Maybe we should handle other functions like this one.

I am not sure whether missing checks of g_raid_destroy_volume() is on purpose 
or ignored,
and those bugs are found by a static analyzer, please advise.
Comment 1 Yuri 2022-04-17 17:47:59 UTC
The advise would be taking static analyzer output with a grain of salt (or a ton, actually) and asking questions on relevant mailing lists instead of using bugtracker when in doubt.

For the report itself, if I understand the code correctly, the vol->v_provider_open != 0 check above all of the g_raid_destroy_volume() calls makes sure all of the code paths returning EBUSY will not be taken.  It would help though to have (void) casts when we are not interested in return values to mark it as "intended".