linux-kernel - Re: [PATCH v2] loop: use vfs_getattr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a8041180-03f2-3342-b568-867b3f295239@huaweicloud.com>
Date: Tue, 12 Aug 2025 10:05:50 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Damien Le Moal <dlemoal@...nel.org>, Rajeev Mishra <rajeevm@....com>,
 axboe@...nel.dk, yukuai1@...weicloud.com
Cc: linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
 "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH v2] loop: use vfs_getattr_nosec() for accurate file size

Hi,

在 2025/08/12 9:40, Damien Le Moal 写道:
> On 8/12/25 4:03 AM, Rajeev Mishra wrote:
>> The get_size() function now uses vfs_getattr_nosec() instead of
>> i_size_read() to obtain file size information. This provides more
>> accurate results for network filesystems where cached metadata
>> may be stale, ensuring the loop device reflects the current file
>> size rather than potentially outdated cached values.
>>
>> Signed-off-by: Rajeev Mishra <rajeevm@....com>
>> ---
>>   drivers/block/loop.c | 24 ++++++++++++++++++++++--
>>   1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
>> index 1b6ee91f8eb9..c418c47db76e 100644
>> --- a/drivers/block/loop.c
>> +++ b/drivers/block/loop.c
>> @@ -137,12 +137,32 @@ static void loop_global_unlock(struct loop_device *lo, bool global)
>>   static int max_part;
>>   static int part_shift;
>>   
>> +/**
>> + * get_size - calculate the effective size of a loop device
>> + * @offset: offset into the backing file
>> + * @sizelimit: user-specified size limit
>> + * @file: the backing file
>> + *
>> + * Calculate the effective size of the loop device
>> + *
>> + * Returns: size in 512-byte sectors, or 0 if invalid
>> + */
>>   static loff_t get_size(loff_t offset, loff_t sizelimit, struct file *file)
>>   {
>> +	struct kstat stat;
>>   	loff_t loopsize;
>> +	int ret;
>> +
>> +	/*
>> +	 * Get the accurate file size. This will prevent caching
>> +	 * issue that occurs at filesystem layer.
>> +	 */
>> +	ret = vfs_getattr_nosec(&file->f_path, &stat, STATX_SIZE, 0);
>> +	if (ret)
>> +		return 0;
> 
> return 0 here is odd. Why not "return ret;" to propagate the error if any ?
> An error may come from the underlying FS inode->i_op->getattr().

This helper is supposed to return the size, all the callers don't
hanldle error value for now.

However, I agree return error value instead of set disk size to 0 will
make more sense, at least from ioctl path. Perhaps go through all the
callers and see, if we want to handle errors or set disk size to 0.

Thanks,
Kuai

> 
>> +
>> +	loopsize = stat.size;
>>   
>> -	/* Compute loopsize in bytes */
>> -	loopsize = i_size_read(file->f_mapping->host);
>>   	if (offset > 0)
>>   		loopsize -= offset;
>>   	/* offset is beyond i_size, weird but possible */
> 
>