* [PATCH] Btrfs: fix infinite loop when tree log recovery
@ 2016-10-07 9:30 robbieko
2016-11-30 16:53 ` Filipe Manana
0 siblings, 1 reply; 5+ messages in thread
From: robbieko @ 2016-10-07 9:30 UTC (permalink / raw)
To: linux-btrfs; +Cc: Robbie Ko
From: Robbie Ko <robbieko@synology.com>
if log tree like below:
leaf N:
...
item 240 key (282 DIR_LOG_ITEM 0) itemoff 8189 itemsize 8
dir log end 1275809046
leaf N+1:
item 0 key (282 DIR_LOG_ITEM 3936149215) itemoff 16275 itemsize 8
dir log end 18446744073709551615
...
when start_ret > 1275809046, but slot[0] never >= nritems,
so never go to next leaf.
Signed-off-by: Robbie Ko <robbieko@synology.com>
---
fs/btrfs/tree-log.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index ef9c55b..e63dd99 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -1940,12 +1940,11 @@ static noinline int find_dir_range(struct btrfs_root *root,
next:
/* check the next slot in the tree to see if it is a valid item */
nritems = btrfs_header_nritems(path->nodes[0]);
+ path->slots[0]++;
if (path->slots[0] >= nritems) {
ret = btrfs_next_leaf(root, path);
if (ret)
goto out;
- } else {
- path->slots[0]++;
}
btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Btrfs: fix infinite loop when tree log recovery
2016-10-07 9:30 [PATCH] Btrfs: fix infinite loop when tree log recovery robbieko
@ 2016-11-30 16:53 ` Filipe Manana
2016-12-01 1:42 ` robbieko
0 siblings, 1 reply; 5+ messages in thread
From: Filipe Manana @ 2016-11-30 16:53 UTC (permalink / raw)
To: robbieko; +Cc: linux-btrfs
On Fri, Oct 7, 2016 at 10:30 AM, robbieko <robbieko@synology.com> wrote:
> From: Robbie Ko <robbieko@synology.com>
>
> if log tree like below:
> leaf N:
> ...
> item 240 key (282 DIR_LOG_ITEM 0) itemoff 8189 itemsize 8
> dir log end 1275809046
> leaf N+1:
> item 0 key (282 DIR_LOG_ITEM 3936149215) itemoff 16275 itemsize 8
> dir log end 18446744073709551615
> ...
>
> when start_ret > 1275809046, but slot[0] never >= nritems,
> so never go to next leaf.
This doesn't explain how the infinite loop happens. Nor exactly how
any problem happens.
It's important to have detailed information in the change logs. I
understand that english isn't your native tongue (it's not mine
either, and I'm far from mastering it), but that's not an excuse to
not express all the important information in detail (we can all live
with grammar errors and typos, and we all do such errors frequently).
I've added this patch to my branch at
https://git.kernel.org/cgit/linux/kernel/git/fdmanana/linux.git/log/?h=for-chris-4.10
but with a modified changelog and subject.
The results of the wrong logic that decides when to move to the next
leaf are unpredictable, and it won't always result in an infinite
loop. We are accessing a slot that doesn't point to an item, to a
memory location containing garbage to something unexpected, and in the
worst case that location is beyond the last page of the extent buffer.
Thanks.
>
> Signed-off-by: Robbie Ko <robbieko@synology.com>
> ---
> fs/btrfs/tree-log.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index ef9c55b..e63dd99 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -1940,12 +1940,11 @@ static noinline int find_dir_range(struct btrfs_root *root,
> next:
> /* check the next slot in the tree to see if it is a valid item */
> nritems = btrfs_header_nritems(path->nodes[0]);
> + path->slots[0]++;
> if (path->slots[0] >= nritems) {
> ret = btrfs_next_leaf(root, path);
> if (ret)
> goto out;
> - } else {
> - path->slots[0]++;
> }
>
> btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Filipe David Manana,
"People will forget what you said,
people will forget what you did,
but people will never forget how you made them feel."
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Btrfs: fix infinite loop when tree log recovery
2016-11-30 16:53 ` Filipe Manana
@ 2016-12-01 1:42 ` robbieko
2016-12-01 11:14 ` Filipe Manana
0 siblings, 1 reply; 5+ messages in thread
From: robbieko @ 2016-12-01 1:42 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
Hi Filipe,
Thank you for your review.
I have seen your modified change log with below
Btrfs: fix tree search logic when replaying directory entry deletes
Btrfs: fix deadlock caused by fsync when logging directory entries
Btrfs: fix enospc in hole punching
So what's the next step ?
modify patch change log and then send again ?
Thanks.
robbieko
Filipe Manana 於 2016-12-01 00:53 寫到:
> On Fri, Oct 7, 2016 at 10:30 AM, robbieko <robbieko@synology.com>
> wrote:
>> From: Robbie Ko <robbieko@synology.com>
>>
>> if log tree like below:
>> leaf N:
>> ...
>> item 240 key (282 DIR_LOG_ITEM 0) itemoff 8189 itemsize 8
>> dir log end 1275809046
>> leaf N+1:
>> item 0 key (282 DIR_LOG_ITEM 3936149215) itemoff 16275
>> itemsize 8
>> dir log end 18446744073709551615
>> ...
>>
>> when start_ret > 1275809046, but slot[0] never >= nritems,
>> so never go to next leaf.
>
> This doesn't explain how the infinite loop happens. Nor exactly how
> any problem happens.
>
> It's important to have detailed information in the change logs. I
> understand that english isn't your native tongue (it's not mine
> either, and I'm far from mastering it), but that's not an excuse to
> not express all the important information in detail (we can all live
> with grammar errors and typos, and we all do such errors frequently).
>
> I've added this patch to my branch at
> https://git.kernel.org/cgit/linux/kernel/git/fdmanana/linux.git/log/?h=for-chris-4.10
> but with a modified changelog and subject.
>
> The results of the wrong logic that decides when to move to the next
> leaf are unpredictable, and it won't always result in an infinite
> loop. We are accessing a slot that doesn't point to an item, to a
> memory location containing garbage to something unexpected, and in the
> worst case that location is beyond the last page of the extent buffer.
>
> Thanks.
>
>
>>
>> Signed-off-by: Robbie Ko <robbieko@synology.com>
>> ---
>> fs/btrfs/tree-log.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
>> index ef9c55b..e63dd99 100644
>> --- a/fs/btrfs/tree-log.c
>> +++ b/fs/btrfs/tree-log.c
>> @@ -1940,12 +1940,11 @@ static noinline int find_dir_range(struct
>> btrfs_root *root,
>> next:
>> /* check the next slot in the tree to see if it is a valid
>> item */
>> nritems = btrfs_header_nritems(path->nodes[0]);
>> + path->slots[0]++;
>> if (path->slots[0] >= nritems) {
>> ret = btrfs_next_leaf(root, path);
>> if (ret)
>> goto out;
>> - } else {
>> - path->slots[0]++;
>> }
>>
>> btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
>> in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Btrfs: fix infinite loop when tree log recovery
2016-12-01 1:42 ` robbieko
@ 2016-12-01 11:14 ` Filipe Manana
2016-12-02 1:32 ` robbieko
0 siblings, 1 reply; 5+ messages in thread
From: Filipe Manana @ 2016-12-01 11:14 UTC (permalink / raw)
To: robbieko; +Cc: linux-btrfs
On Thu, Dec 1, 2016 at 1:42 AM, robbieko <robbieko@synology.com> wrote:
> Hi Filipe,
>
> Thank you for your review.
> I have seen your modified change log with below
> Btrfs: fix tree search logic when replaying directory entry deletes
> Btrfs: fix deadlock caused by fsync when logging directory entries
> Btrfs: fix enospc in hole punching
> So what's the next step ?
> modify patch change log and then send again ?
You don't need to do anything else for those patches.
Thanks.
>
> Thanks.
> robbieko
>
> Filipe Manana 於 2016-12-01 00:53 寫到:
>
>> On Fri, Oct 7, 2016 at 10:30 AM, robbieko <robbieko@synology.com> wrote:
>>>
>>> From: Robbie Ko <robbieko@synology.com>
>>>
>>> if log tree like below:
>>> leaf N:
>>> ...
>>> item 240 key (282 DIR_LOG_ITEM 0) itemoff 8189 itemsize 8
>>> dir log end 1275809046
>>> leaf N+1:
>>> item 0 key (282 DIR_LOG_ITEM 3936149215) itemoff 16275 itemsize 8
>>> dir log end 18446744073709551615
>>> ...
>>>
>>> when start_ret > 1275809046, but slot[0] never >= nritems,
>>> so never go to next leaf.
>>
>>
>> This doesn't explain how the infinite loop happens. Nor exactly how
>> any problem happens.
>>
>> It's important to have detailed information in the change logs. I
>> understand that english isn't your native tongue (it's not mine
>> either, and I'm far from mastering it), but that's not an excuse to
>> not express all the important information in detail (we can all live
>> with grammar errors and typos, and we all do such errors frequently).
>>
>> I've added this patch to my branch at
>>
>> https://git.kernel.org/cgit/linux/kernel/git/fdmanana/linux.git/log/?h=for-chris-4.10
>> but with a modified changelog and subject.
>>
>> The results of the wrong logic that decides when to move to the next
>> leaf are unpredictable, and it won't always result in an infinite
>> loop. We are accessing a slot that doesn't point to an item, to a
>> memory location containing garbage to something unexpected, and in the
>> worst case that location is beyond the last page of the extent buffer.
>>
>> Thanks.
>>
>>
>>>
>>> Signed-off-by: Robbie Ko <robbieko@synology.com>
>>> ---
>>> fs/btrfs/tree-log.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
>>> index ef9c55b..e63dd99 100644
>>> --- a/fs/btrfs/tree-log.c
>>> +++ b/fs/btrfs/tree-log.c
>>> @@ -1940,12 +1940,11 @@ static noinline int find_dir_range(struct
>>> btrfs_root *root,
>>> next:
>>> /* check the next slot in the tree to see if it is a valid item
>>> */
>>> nritems = btrfs_header_nritems(path->nodes[0]);
>>> + path->slots[0]++;
>>> if (path->slots[0] >= nritems) {
>>> ret = btrfs_next_leaf(root, path);
>>> if (ret)
>>> goto out;
>>> - } else {
>>> - path->slots[0]++;
>>> }
>>>
>>> btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>>> --
>>> 1.9.1
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
>
--
Filipe David Manana,
"People will forget what you said,
people will forget what you did,
but people will never forget how you made them feel."
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Btrfs: fix infinite loop when tree log recovery
2016-12-01 11:14 ` Filipe Manana
@ 2016-12-02 1:32 ` robbieko
0 siblings, 0 replies; 5+ messages in thread
From: robbieko @ 2016-12-02 1:32 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
Hi Filipe,
Thank you for your help.
I will make up the incremental send change log as soon as possible.
Thanks.
robbieko
Filipe Manana 於 2016-12-01 19:14 寫到:
> On Thu, Dec 1, 2016 at 1:42 AM, robbieko <robbieko@synology.com> wrote:
>> Hi Filipe,
>>
>> Thank you for your review.
>> I have seen your modified change log with below
>> Btrfs: fix tree search logic when replaying directory entry
>> deletes
>> Btrfs: fix deadlock caused by fsync when logging directory entries
>> Btrfs: fix enospc in hole punching
>> So what's the next step ?
>> modify patch change log and then send again ?
>
> You don't need to do anything else for those patches.
> Thanks.
>
>>
>> Thanks.
>> robbieko
>>
>> Filipe Manana 於 2016-12-01 00:53 寫到:
>>
>>> On Fri, Oct 7, 2016 at 10:30 AM, robbieko <robbieko@synology.com>
>>> wrote:
>>>>
>>>> From: Robbie Ko <robbieko@synology.com>
>>>>
>>>> if log tree like below:
>>>> leaf N:
>>>> ...
>>>> item 240 key (282 DIR_LOG_ITEM 0) itemoff 8189 itemsize 8
>>>> dir log end 1275809046
>>>> leaf N+1:
>>>> item 0 key (282 DIR_LOG_ITEM 3936149215) itemoff 16275
>>>> itemsize 8
>>>> dir log end 18446744073709551615
>>>> ...
>>>>
>>>> when start_ret > 1275809046, but slot[0] never >= nritems,
>>>> so never go to next leaf.
>>>
>>>
>>> This doesn't explain how the infinite loop happens. Nor exactly how
>>> any problem happens.
>>>
>>> It's important to have detailed information in the change logs. I
>>> understand that english isn't your native tongue (it's not mine
>>> either, and I'm far from mastering it), but that's not an excuse to
>>> not express all the important information in detail (we can all live
>>> with grammar errors and typos, and we all do such errors frequently).
>>>
>>> I've added this patch to my branch at
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/fdmanana/linux.git/log/?h=for-chris-4.10
>>> but with a modified changelog and subject.
>>>
>>> The results of the wrong logic that decides when to move to the next
>>> leaf are unpredictable, and it won't always result in an infinite
>>> loop. We are accessing a slot that doesn't point to an item, to a
>>> memory location containing garbage to something unexpected, and in
>>> the
>>> worst case that location is beyond the last page of the extent
>>> buffer.
>>>
>>> Thanks.
>>>
>>>
>>>>
>>>> Signed-off-by: Robbie Ko <robbieko@synology.com>
>>>> ---
>>>> fs/btrfs/tree-log.c | 3 +--
>>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
>>>> index ef9c55b..e63dd99 100644
>>>> --- a/fs/btrfs/tree-log.c
>>>> +++ b/fs/btrfs/tree-log.c
>>>> @@ -1940,12 +1940,11 @@ static noinline int find_dir_range(struct
>>>> btrfs_root *root,
>>>> next:
>>>> /* check the next slot in the tree to see if it is a valid
>>>> item
>>>> */
>>>> nritems = btrfs_header_nritems(path->nodes[0]);
>>>> + path->slots[0]++;
>>>> if (path->slots[0] >= nritems) {
>>>> ret = btrfs_next_leaf(root, path);
>>>> if (ret)
>>>> goto out;
>>>> - } else {
>>>> - path->slots[0]++;
>>>> }
>>>>
>>>> btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>>>> --
>>>> 1.9.1
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe
>>>> linux-btrfs" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-02 1:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-07 9:30 [PATCH] Btrfs: fix infinite loop when tree log recovery robbieko
2016-11-30 16:53 ` Filipe Manana
2016-12-01 1:42 ` robbieko
2016-12-01 11:14 ` Filipe Manana
2016-12-02 1:32 ` robbieko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.