1. Patchew
  2. linux
  3. View series

[PATCH V3] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate

Lizhi Xu posted 1 patch 1 year, 5 months ago 
fs/ocfs2/buffer_head_io.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH V3] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate
Posted by Lizhi Xu 1 year, 5 months ago 
In the for-loop after the 'read_failure' label, the condition
'(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
When this contidion is true, this for-loop will call ocfs2_set_buffer
_uptodate(ci, bh), which then triggers a NULL pointer access error.

Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/ocfs2/buffer_head_io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index e62c7e1de4eb..8f714406528d 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
 		/* Always set the buffer in the cache, even if it was
 		 * a forced read, or read-ahead which hasn't yet
 		 * completed. */
-		ocfs2_set_buffer_uptodate(ci, bh);
+		if (bh)
+			ocfs2_set_buffer_uptodate(ci, bh);
 	}
 	ocfs2_metadata_cache_io_unlock(ci);
 
-- 
2.43.0
Re: [PATCH V3] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate
Posted by Heming Zhao 1 year, 5 months ago 
Could you resend v3 with both patches?
And add my "Reviewed-by" tag to another patch.

btw, you miss change log area from v2.

-Heming

On 8/20/24 15:37, Lizhi Xu wrote:
> In the for-loop after the 'read_failure' label, the condition
> '(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
> When this contidion is true, this for-loop will call ocfs2_set_buffer
> _uptodate(ci, bh), which then triggers a NULL pointer access error.
> 
> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
>   fs/ocfs2/buffer_head_io.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index e62c7e1de4eb..8f714406528d 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>   		/* Always set the buffer in the cache, even if it was
>   		 * a forced read, or read-ahead which hasn't yet
>   		 * completed. */
> -		ocfs2_set_buffer_uptodate(ci, bh);
> +		if (bh)
> +			ocfs2_set_buffer_uptodate(ci, bh);
>   	}
>   	ocfs2_metadata_cache_io_unlock(ci);
>
[PATCH V3 1/2] ocfs2: remove unreasonable unlock
Posted by Lizhi Xu 1 year, 5 months ago 
There was a lock release before exiting, so remove the unreasonable unlock.

Reported-and-tested-by: syzbot+ab134185af9ef88dfed5@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ab134185af9ef88dfed5
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
---
 fs/ocfs2/buffer_head_io.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index cdb9b9bdea1f..e62c7e1de4eb 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -235,7 +235,6 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
 		if (bhs[i] == NULL) {
 			bhs[i] = sb_getblk(sb, block++);
 			if (bhs[i] == NULL) {
-				ocfs2_metadata_cache_io_unlock(ci);
 				status = -ENOMEM;
 				mlog_errno(status);
 				/* Don't forget to put previous bh! */
-- 
2.43.0
Re: [PATCH V3 1/2] ocfs2: remove unreasonable unlock
Posted by Joseph Qi 1 year, 5 months ago 

On 8/20/24 5:45 PM, Lizhi Xu wrote:
> There was a lock release before exiting, so remove the unreasonable unlock.
> 
> Reported-and-tested-by: syzbot+ab134185af9ef88dfed5@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ab134185af9ef88dfed5
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> Reviewed-by: Heming Zhao <heming.zhao@suse.com>

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
>  fs/ocfs2/buffer_head_io.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index cdb9b9bdea1f..e62c7e1de4eb 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -235,7 +235,6 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>  		if (bhs[i] == NULL) {
>  			bhs[i] = sb_getblk(sb, block++);
>  			if (bhs[i] == NULL) {
> -				ocfs2_metadata_cache_io_unlock(ci);
>  				status = -ENOMEM;
>  				mlog_errno(status);
>  				/* Don't forget to put previous bh! */
[PATCH V3 2/2] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate
Posted by Lizhi Xu 1 year, 5 months ago 
In the for-loop after the 'read_failure' label, the condition
'(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
When this contidion is true, this for-loop will call ocfs2_set_buffer
_uptodate(ci, bh), which then triggers a NULL pointer access error.

Changes from V2:
* Make the code more concise

Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
---
 fs/ocfs2/buffer_head_io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index e62c7e1de4eb..8f714406528d 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
 		/* Always set the buffer in the cache, even if it was
 		 * a forced read, or read-ahead which hasn't yet
 		 * completed. */
-		ocfs2_set_buffer_uptodate(ci, bh);
+		if (bh)
+			ocfs2_set_buffer_uptodate(ci, bh);
 	}
 	ocfs2_metadata_cache_io_unlock(ci);
 
-- 
2.43.0
Re: [PATCH V3 2/2] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate
Posted by Joseph Qi 1 year, 5 months ago 

On 8/20/24 5:45 PM, Lizhi Xu wrote:
> In the for-loop after the 'read_failure' label, the condition
> '(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
> When this contidion is true, this for-loop will call ocfs2_set_buffer
> _uptodate(ci, bh), which then triggers a NULL pointer access error.
> 

Or it may simplified as the following:

When doing cleanup, if flags without OCFS2_BH_READAHEAD, it may trigger
NULL pointer dereference in the following ocfs2_set_buffer_uptodate() if
bh is NULL.

> Changes from V2:
> * Make the code more concise
> 

This is not the right place for changelog.

Thanks,
Joseph

> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
>  fs/ocfs2/buffer_head_io.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index e62c7e1de4eb..8f714406528d 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>  		/* Always set the buffer in the cache, even if it was
>  		 * a forced read, or read-ahead which hasn't yet
>  		 * completed. */
> -		ocfs2_set_buffer_uptodate(ci, bh);
> +		if (bh)
> +			ocfs2_set_buffer_uptodate(ci, bh);
>  	}
>  	ocfs2_metadata_cache_io_unlock(ci);
>
Re: [PATCH V3 2/2] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate
Posted by Joseph Qi 1 year, 5 months ago 
And this is not a UAF case, but NULL pointer dereference.
So I suggest change the subject to:
ocfs2: fix possible NULL pointer dereference in ocfs2_set_buffer_uptodate

On 8/21/24 8:08 AM, Joseph Qi wrote:
> 
> 
> On 8/20/24 5:45 PM, Lizhi Xu wrote:
>> In the for-loop after the 'read_failure' label, the condition
>> '(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
>> When this contidion is true, this for-loop will call ocfs2_set_buffer
>> _uptodate(ci, bh), which then triggers a NULL pointer access error.
>>
> 
> Or it may simplified as the following:
> 
> When doing cleanup, if flags without OCFS2_BH_READAHEAD, it may trigger
> NULL pointer dereference in the following ocfs2_set_buffer_uptodate() if
> bh is NULL.
> 
>> Changes from V2:
>> * Make the code more concise
>>
> 
> This is not the right place for changelog.
> 
> Thanks,
> Joseph
> 
>> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
>> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
>> Reviewed-by: Heming Zhao <heming.zhao@suse.com>
>> ---
>>  fs/ocfs2/buffer_head_io.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
>> index e62c7e1de4eb..8f714406528d 100644
>> --- a/fs/ocfs2/buffer_head_io.c
>> +++ b/fs/ocfs2/buffer_head_io.c
>> @@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>>  		/* Always set the buffer in the cache, even if it was
>>  		 * a forced read, or read-ahead which hasn't yet
>>  		 * completed. */
>> -		ocfs2_set_buffer_uptodate(ci, bh);
>> +		if (bh)
>> +			ocfs2_set_buffer_uptodate(ci, bh);
>>  	}
>>  	ocfs2_metadata_cache_io_unlock(ci);
>>
Re: [PATCH V3 2/2] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate
Posted by Heming Zhao 1 year, 5 months ago 
On 8/21/24 10:34, Joseph Qi wrote:
> And this is not a UAF case, but NULL pointer dereference.
> So I suggest change the subject to:
> ocfs2: fix possible NULL pointer dereference in ocfs2_set_buffer_uptodate

I agree with above too.
I didn't care about the patch subject in previous review jobs, 'UAF' is not suitable.

-Heming

> 
> On 8/21/24 8:08 AM, Joseph Qi wrote:
>>
>>
>> On 8/20/24 5:45 PM, Lizhi Xu wrote:
>>> In the for-loop after the 'read_failure' label, the condition
>>> '(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
>>> When this contidion is true, this for-loop will call ocfs2_set_buffer
>>> _uptodate(ci, bh), which then triggers a NULL pointer access error.
>>>
>>
>> Or it may simplified as the following:
>>
>> When doing cleanup, if flags without OCFS2_BH_READAHEAD, it may trigger
>> NULL pointer dereference in the following ocfs2_set_buffer_uptodate() if
>> bh is NULL.
>>
>>> Changes from V2:
>>> * Make the code more concise
>>>
>>
>> This is not the right place for changelog.
>>
>> Thanks,
>> Joseph
>>
>>> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
>>> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
>>> Reviewed-by: Heming Zhao <heming.zhao@suse.com>
>>> ---
>>>   fs/ocfs2/buffer_head_io.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
>>> index e62c7e1de4eb..8f714406528d 100644
>>> --- a/fs/ocfs2/buffer_head_io.c
>>> +++ b/fs/ocfs2/buffer_head_io.c
>>> @@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>>>   		/* Always set the buffer in the cache, even if it was
>>>   		 * a forced read, or read-ahead which hasn't yet
>>>   		 * completed. */
>>> -		ocfs2_set_buffer_uptodate(ci, bh);
>>> +		if (bh)
>>> +			ocfs2_set_buffer_uptodate(ci, bh);
>>>   	}
>>>   	ocfs2_metadata_cache_io_unlock(ci);
>>>
Re: [PATCH V3 2/2] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate
Posted by Lizhi Xu 1 year, 5 months ago 
On Wed, 21 Aug 2024 10:39:39 +0800, Heming Zhao wrote:
> > And this is not a UAF case, but NULL pointer dereference.
> > So I suggest change the subject to:
> > ocfs2: fix possible NULL pointer dereference in ocfs2_set_buffer_uptodate
> 
> I agree with above too.
> I didn't care about the patch subject in previous review jobs, 'UAF' is not suitable.
> 
> -Heming
OK, I will update and send this patch V4 separately.

Lizhi
[PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate
Posted by Lizhi Xu 1 year, 5 months ago 
When doing cleanup, if flags without OCFS2_BH_READAHEAD, it may trigger
NULL pointer dereference in the following ocfs2_set_buffer_uptodate() if
bh is NULL.

Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
V3 -> V4: Update comments and subject

 fs/ocfs2/buffer_head_io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index e62c7e1de4eb..8f714406528d 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
 		/* Always set the buffer in the cache, even if it was
 		 * a forced read, or read-ahead which hasn't yet
 		 * completed. */
-		ocfs2_set_buffer_uptodate(ci, bh);
+		if (bh)
+			ocfs2_set_buffer_uptodate(ci, bh);
 	}
 	ocfs2_metadata_cache_io_unlock(ci);
 
-- 
2.43.0
Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate
Posted by Joseph Qi 1 year, 5 months ago 

On 8/21/24 2:14 PM, Lizhi Xu wrote:
> When doing cleanup, if flags without OCFS2_BH_READAHEAD, it may trigger
> NULL pointer dereference in the following ocfs2_set_buffer_uptodate() if
> bh is NULL.
> 
> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
> V3 -> V4: Update comments and subject
> 
>  fs/ocfs2/buffer_head_io.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index e62c7e1de4eb..8f714406528d 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>  		/* Always set the buffer in the cache, even if it was
>  		 * a forced read, or read-ahead which hasn't yet
>  		 * completed. */
> -		ocfs2_set_buffer_uptodate(ci, bh);
> +		if (bh)
> +			ocfs2_set_buffer_uptodate(ci, bh);
>  	}
>  	ocfs2_metadata_cache_io_unlock(ci);
>
Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate
Posted by heming.zhao@suse.com 1 year, 5 months ago 
Hi,

Where is my "Reviewed-by" tag, and where is [patch 1/2]?

On 8/21/24 14:14, Lizhi Xu wrote:
> When doing cleanup, if flags without OCFS2_BH_READAHEAD, it may trigger
> NULL pointer dereference in the following ocfs2_set_buffer_uptodate() if
> bh is NULL.
> 
> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
> V3 -> V4: Update comments and subject> 
>   fs/ocfs2/buffer_head_io.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index e62c7e1de4eb..8f714406528d 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>   		/* Always set the buffer in the cache, even if it was
>   		 * a forced read, or read-ahead which hasn't yet
>   		 * completed. */
> -		ocfs2_set_buffer_uptodate(ci, bh);
> +		if (bh)
> +			ocfs2_set_buffer_uptodate(ci, bh);
>   	}
>   	ocfs2_metadata_cache_io_unlock(ci);
>
Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate
Posted by Lizhi Xu 1 year, 5 months ago 
On Wed, 21 Aug 2024 14:23:08 +0800, Heming Zhao wrote:
> Where is my "Reviewed-by" tag, and where is [patch 1/2]?
Sorry about your "Reviewed-by" tag, I remove it, if you don't mind, you can
add it by yourself.

In my previous email, I explicitly stated that only this patch should
be sent separately, as the first patch has already been reviewed by two
reviewers. If the second patch is updated with the first patch, I
personally think it is unnecessary.

[patch 1/2]: https://lore.kernel.org/all/20240820094512.2228159-1-lizhi.xu@windriver.com/ 

Lizhi
Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate
Posted by heming.zhao@suse.com 1 year, 5 months ago 
Hi,

On 8/21/24 14:55, Lizhi Xu wrote:
> On Wed, 21 Aug 2024 14:23:08 +0800, Heming Zhao wrote:
>> Where is my "Reviewed-by" tag, and where is [patch 1/2]?
> Sorry about your "Reviewed-by" tag, I remove it, if you don't mind, you can
> add it by yourself.

Good answer!

This patch issue was found by me, and I also pointed out how to fix it, then take the time
to review your code. But in the end, you removed my "Reviewed-by" tag.

> 
> In my previous email, I explicitly stated that only this patch should
> be sent separately, as the first patch has already been reviewed by two
> reviewers. If the second patch is updated with the first patch, I
> personally think it is unnecessary.
> 
> [patch 1/2]: https://lore.kernel.org/all/20240820094512.2228159-1-lizhi.xu@windriver.com/
> 
> Lizhi

It looks like you don't have basis knowledge of how to send patches.

I will never reply to or review any of your mails/patches.
Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate
Posted by Joseph Qi 1 year, 5 months ago 

On 8/21/24 3:37 PM, heming.zhao@suse.com wrote:
> Hi,
> 
> On 8/21/24 14:55, Lizhi Xu wrote:
>> On Wed, 21 Aug 2024 14:23:08 +0800, Heming Zhao wrote:
>>> Where is my "Reviewed-by" tag, and where is [patch 1/2]?
>> Sorry about your "Reviewed-by" tag, I remove it, if you don't mind, you can
>> add it by yourself.
> 
> Good answer!
> 
> This patch issue was found by me, and I also pointed out how to fix it, then take the time
> to review your code. But in the end, you removed my "Reviewed-by" tag.
> 

Seems a misunderstanding, take it easy:) 
Lizhi may think since this is a new version, it needs a new round review.

>>
>> In my previous email, I explicitly stated that only this patch should
>> be sent separately, as the first patch has already been reviewed by two
>> reviewers. If the second patch is updated with the first patch, I
>> personally think it is unnecessary.
>>
>> [patch 1/2]: https://lore.kernel.org/all/20240820094512.2228159-1-lizhi.xu@windriver.com/
>>
>> Lizhi
> 
> It looks like you don't have basis knowledge of how to send patches.
> 
> I will never reply to or review any of your mails/patches.
Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate
Posted by Lizhi Xu 1 year, 5 months ago 
On Wed, 21 Aug 2024 15:58:36 +0800, Joseph Qi wrote:
> On 8/21/24 3:37 PM, heming.zhao@suse.com wrote:
> > Hi,
> >
> > On 8/21/24 14:55, Lizhi Xu wrote:
> >> On Wed, 21 Aug 2024 14:23:08 +0800, Heming Zhao wrote:
> >>> Where is my "Reviewed-by" tag, and where is [patch 1/2]?
> >> Sorry about your "Reviewed-by" tag, I remove it, if you don't mind, you can
> >> add it by yourself.
> >
> > Good answer!
> >
> > This patch issue was found by me, and I also pointed out how to fix it, then take the time
> > to review your code. But in the end, you removed my "Reviewed-by" tag.
> >
> 
> Seems a misunderstanding, take it easy:)
> Lizhi may think since this is a new version, it needs a new round review.
Yeah, the subject and comments have all been changed.
Thank you for defending me:)

BR,
Lizhi
Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate
Posted by Heming Zhao 1 year, 5 months ago 
On 8/21/24 17:14, Lizhi Xu wrote:
> On Wed, 21 Aug 2024 15:58:36 +0800, Joseph Qi wrote:
>> On 8/21/24 3:37 PM, heming.zhao@suse.com wrote:
>>> Hi,
>>>
>>> On 8/21/24 14:55, Lizhi Xu wrote:
>>>> On Wed, 21 Aug 2024 14:23:08 +0800, Heming Zhao wrote:
>>>>> Where is my "Reviewed-by" tag, and where is [patch 1/2]?
>>>> Sorry about your "Reviewed-by" tag, I remove it, if you don't mind, you can
>>>> add it by yourself.
>>>
>>> Good answer!
>>>
>>> This patch issue was found by me, and I also pointed out how to fix it, then take the time
>>> to review your code. But in the end, you removed my "Reviewed-by" tag.
>>>
>>
>> Seems a misunderstanding, take it easy:)
>> Lizhi may think since this is a new version, it needs a new round review.
> Yeah, the subject and comments have all been changed.
> Thank you for defending me:)

OK.

> 
> BR,
> Lizhi
Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate
Posted by Andrew Morton 1 year, 5 months ago 
OK I think I found the correct patches - v3 of [1/2] and v4 of [2/2].

For clarity can we please have a full resend of both patches?

And let's please have a [0/n] cover letter which describes the problems
which are being addressed and which also briefly describes how they
were addressed.

Also, it appears that both of these fixes should be backported into
-stable kernels.  So let's please try to identify when these bugs were
introduced and to add a suitable Fixes: to the individual changelogs.

Thanks.
Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate
Posted by Andrew Morton 1 year, 5 months ago 
On Wed, 21 Aug 2024 14:39:11 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> OK I think I found the correct patches - v3 of [1/2] and v4 of [2/2].
> 
> For clarity can we please have a full resend of both patches?
> 
> And let's please have a [0/n] cover letter which describes the problems
> which are being addressed and which also briefly describes how they
> were addressed.
> 
> Also, it appears that both of these fixes should be backported into
> -stable kernels.  So let's please try to identify when these bugs were
> introduced and to add a suitable Fixes: to the individual changelogs.
> 

Again, can we please have a full resend of these two patches with the
above issues addressed?  Particularly the identification of the Fixes:
targets.

Thanks.
Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate
Posted by Heming Zhao 1 year, 5 months ago 
On 9/2/24 08:54, Andrew Morton wrote:
> On Wed, 21 Aug 2024 14:39:11 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> 
>> OK I think I found the correct patches - v3 of [1/2] and v4 of [2/2].
>>
>> For clarity can we please have a full resend of both patches?
>>
>> And let's please have a [0/n] cover letter which describes the problems
>> which are being addressed and which also briefly describes how they
>> were addressed.
>>
>> Also, it appears that both of these fixes should be backported into
>> -stable kernels.  So let's please try to identify when these bugs were
>> introduced and to add a suitable Fixes: to the individual changelogs.
>>
> 
> Again, can we please have a full resend of these two patches with the
> above issues addressed?  Particularly the identification of the Fixes:
> targets.
> 
> Thanks.

Hello Andrew & Joseph,

If Lizhi still doesn't respond by this Friday, I will send his latest patch set again.

-Heming
Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate
Posted by Joseph Qi 1 year, 5 months ago 

On 9/2/24 9:03 AM, Heming Zhao wrote:
> On 9/2/24 08:54, Andrew Morton wrote:
>> On Wed, 21 Aug 2024 14:39:11 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
>>
>>> OK I think I found the correct patches - v3 of [1/2] and v4 of [2/2].
>>>
>>> For clarity can we please have a full resend of both patches?
>>>
>>> And let's please have a [0/n] cover letter which describes the problems
>>> which are being addressed and which also briefly describes how they
>>> were addressed.
>>>
>>> Also, it appears that both of these fixes should be backported into
>>> -stable kernels.  So let's please try to identify when these bugs were
>>> introduced and to add a suitable Fixes: to the individual changelogs.
>>>
>>
>> Again, can we please have a full resend of these two patches with the
>> above issues addressed?  Particularly the identification of the Fixes:
>> targets.
>>
>> Thanks.
> 
> Hello Andrew & Joseph,
> 
> If Lizhi still doesn't respond by this Friday, I will send his latest patch set again.
> 
I'll do that, thanks.

Joseph
Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate
Posted by Lizhi Xu 1 year, 5 months ago 
On Mon, 2 Sep 2024 10:20:38 +0800, Joseph Qi wrote:
>On 9/2/24 9:03 AM, Heming Zhao wrote:
>> On 9/2/24 08:54, Andrew Morton wrote:
>>> On Wed, 21 Aug 2024 14:39:11 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
>>>
>>>> OK I think I found the correct patches - v3 of [1/2] and v4 of [2/2].
>>>>
>>>> For clarity can we please have a full resend of both patches?
>>>>
>>>> And let's please have a [0/n] cover letter which describes the problems
>>>> which are being addressed and which also briefly describes how they
>>>> were addressed.
>>>>
>>>> Also, it appears that both of these fixes should be backported into
>>>> -stable kernels.  So let's please try to identify when these bugs were
>>>> introduced and to add a suitable Fixes: to the individual changelogs.
>>>>
>>>
>>> Again, can we please have a full resend of these two patches with the
>>> above issues addressed?  Particularly the identification of the Fixes:
>>> targets.
>>>
>>> Thanks.
>>
>> Hello Andrew & Joseph,
>>
>> If Lizhi still doesn't respond by this Friday, I will send his latest patch set again.
>>
>I'll do that, thanks.
Sorry, I didn't notice these emails. Thanks for your help.

BR,
Lizhi
Re: [PATCH V3 2/2] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate
Posted by Heming Zhao 1 year, 5 months ago 
On 8/20/24 17:45, Lizhi Xu wrote:
> In the for-loop after the 'read_failure' label, the condition
> '(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
> When this contidion is true, this for-loop will call ocfs2_set_buffer
> _uptodate(ci, bh), which then triggers a NULL pointer access error.
> 
> Changes from V2:
> * Make the code more concise
> 
> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> Reviewed-by: Heming Zhao <heming.zhao@suse.com>

I didn't give you my "Reviewed-by" tag for this patch, and you
can add my tag only after I send it to you.
(take easy, you can get my "Reviewed-by" tag now.)
Please remember this rule for next time.

Another issue with this mail is that the change log should be
placed before the file list, not in the commit message section.

ref: Documentation/process/submitting-patches.rst

Thanks,
Heming

> ---
>   fs/ocfs2/buffer_head_io.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index e62c7e1de4eb..8f714406528d 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>   		/* Always set the buffer in the cache, even if it was
>   		 * a forced read, or read-ahead which hasn't yet
>   		 * completed. */
> -		ocfs2_set_buffer_uptodate(ci, bh);
> +		if (bh)
> +			ocfs2_set_buffer_uptodate(ci, bh);
>   	}
>   	ocfs2_metadata_cache_io_unlock(ci);
>
Re: [PATCH] ocfs2: remove unreasonable unlock
Posted by Lizhi Xu 1 year, 5 months ago 
On Tue, 20 Aug 2024 19:32:03 +0800, Heming wrote:
> > In the for-loop after the 'read_failure' label, the condition
> > '(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
> > When this contidion is true, this for-loop will call ocfs2_set_buffer
> > _uptodate(ci, bh), which then triggers a NULL pointer access error.
> >
> > Changes from V2:
> > * Make the code more concise
> >
> > Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
> > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> 
> I didn't give you my "Reviewed-by" tag for this patch, and you
> can add my tag only after I send it to you.
> (take easy, you can get my "Reviewed-by" tag now.)
> Please remember this rule for next time.
Got it.
> 
> Another issue with this mail is that the change log should be
> placed before the file list, not in the commit message section.
Thanks. Its like following:
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
V2 -> V3: Make the code more concise

 fs/ocfs2/buffer_head_io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

BR,
Lizhi

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.