On 2021/8/31 11:02, Theodore Ts'o wrote:
> On Thu, Aug 26, 2021 at 09:04:12PM +0800, Zhang Yi wrote:
>>
>> So this patch initialize the inode buffer by filling the in-mem inode
>> contents if we skip read I/O, ensure that the buffer is really uptodate.
>>
>> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
>> ---
>> fs/ext4/inode.c | 22 ++++++++++++++++------
>> 1 file changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 3c36e701e30e..8b37f55b04ad 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -4446,8 +4446,8 @@ static int ext4_fill_raw_inode(struct inode *inode, struct ext4_inode *raw_inode
>> * inode.
>> */
>> static int __ext4_get_inode_loc(struct super_block *sb, unsigned long ino,
>> - struct ext4_iloc *iloc, int in_mem,
>> - ext4_fsblk_t *ret_block)
>> + struct inode *inode, struct ext4_iloc *iloc,
>> + int in_mem, ext4_fsblk_t *ret_block)
>
>
> In this patch you've added a new argument 'inode'. However, if in_mem
> is true, and inode is NULL, the kernel will crash with a null pointer
> dereference. Furthermore, whenever in_mem is false, the callers pass
> in NULL for inode.
>
> Given that, perhaps we should just drop the in_mem argument, and then
> instead of
>
> if (in_mem) {
>
> we do:
>
> if (inode && !ext4_test_inode_state(inode, EXT4_STATE_XATTR) {
>
> with the comments adjusted accordingly?
>
> I think it will make the code a bit simpler and readable.
>
> What do you think?
>
Yes，although I have already prevent passing 'in_mem' is true but 'inode' is
NULL in ext4_get_inode_loc(), using two arguments show the inode in-mem case
is not safe. I will remove the 'in_mem' parameter.
Thanks,
Yi.
