On 19.12.2017 08:05, Misono, Tomohiro wrote:
> Hello,
>
> On 2017/12/18 19:06, Nikolay Borisov wrote:
>>
>>
>> On 18.12.2017 12:03, Nikolay Borisov wrote:
>>> Currently if a mounted-btrfs instance is mounted for the 2nd time
>>> without first unmounting the first instance then we hit a memory leak
>>> in btrfs_mount_root due to the fs_info of the acquired superblock is
>>> different than the newly allocated fs info. Fix this by specifically
>>> checking if the fs_info instance of the newly acquired superblock is
>>> the same as ours and free it if not.
>>>
>>> Reproducer:
>>>
>>> mount /dev/vdc /media/scratch
>>> mount /dev/vdc /media/scratch2 <- memory leak hit
>>>
>>> Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx>
>>
>> This one fixes:
>>
>> 4da82a9e60a1 btrfs: add btrfs_mount_root() and new file_system_type
>>
>> Which seems to be in for-next only.
>
> The patch doesn't change the logic here (same as current btrfs_mount()).
>
>>
>>> ---
>>> fs/btrfs/super.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>>> index e57eb6e70278..ea3bca85be44 100644
>>> --- a/fs/btrfs/super.c
>>> +++ b/fs/btrfs/super.c
>>> @@ -1612,7 +1612,11 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
>>> goto error_sec_opts;
>>> }
>>>
>>> - fs_info = btrfs_sb(s);
>>> + if (btrfs_sb(s) != fs_info) {
>>> + free_fs_info(fs_info);
>>> + fs_info = btrfs_sb(s);
>>> + }
>>> +
>>> error = setup_security_options(fs_info, s, &new_sec_opts);
>>> if (error) {
>>> deactivate_locked_super(s);
>>>
>
> I'm not sure how the memory leak happens.
> Just above this line is:
>
> ---
> if (s->s_root) {
> btrfs_close_devices(fs_devices);
> free_fs_info(fs_info);
> if ((flags ^ s->s_flags) & SB_RDONLY)
> error = -EBUSY;
> } else {
> snprintf(s->s_id, sizeof(s->s_id), "%pg", bdev);
> btrfs_sb(s)->bdev_holder = fs_type;
> error = btrfs_fill_super(s, fs_devices, data);
> }
> ---
>
> In the first mount, s->s_root is null and btrfs_fill_super() is called.
> On the other hand, in the second mount, s->s_root is not null and fs_info is freed there.
>
> So, I think there is no memory leak in the current code, or am I missing something?
Hm, you are right, I've missed that, so disregard this patch.
>
> Regards,
> Tomohiro
>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
