Question: consult patch

Question: consult patch

[zhangwensheng (E)]

Hi Lee ：
     I saw your patch because of CVE-2022-20158, the patch like below:

---
     mm: backing-dev: Take a reference to the bdi in use to prevent UAF

     KASAN reports a reproducible issue in the BDI handling code due to a
     dereference of a previously freed pointer to 'struct backing_dev_info'
     in bdi_unregister().

     Because of a distinct lack of locking and/or reference taking,
     blk_cleanup_queue() puts the final taken reference to the bdi, which
     is then promptly freed by release_bdi().  However, del_gendisk() calls
     bdi_unregister() after the fact, which then attempts to dereference
     it causing the kernel to panic.

     Bug: 182815710
     Signed-off-by: Lee Jones <lee.jones@linaro.org>
     Change-Id: Iaf3dadf3b983a4b7d74d4e273fc676350cfc387f
---

     I am sorry that I have some questions for this patch. According to
     my analysis, I failed to find the relevant process may lead to null
     pointer reference. can you show me "KASAN reports a reproducible issue"
     that what the problem is?  thank you very much!

Re: Question: consult patch

[Christoph Hellwig]

On Fri, Aug 12, 2022 at 11:34:59AM +0800, zhangwensheng (E) wrote:
> Hi Lee ：
>     I saw your patch because of CVE-2022-20158, the patch like below:
> 
> ---
>     mm: backing-dev: Take a reference to the bdi in use to prevent UAF

I can't see that patch anywhere, and I've not seen an bug report for it.

>     Because of a distinct lack of locking and/or reference taking,
>     blk_cleanup_queue() puts the final taken reference to the bdi, which

.. and blk_cleanup_queue also is gone upstream.

What am I missing?

Re: Question: consult patch

[zhangwensheng (E)]

Hi

In CVE list last week, there is a new cve reported in asop 4.14 like below:
Reference link: 
https://lore.kernel.org/all/CAODzB9rgMexvLjE=WuTm+SN8SfUggaZgWG-aBcy6cotppju6mw@mail.gmail.com/T/

---
CVE-2022-20158: mm: backing-dev: Take a reference to the bdi in use to
prevent UAF

CVSS v3 score is not assigned.

AOSP kernel 4.14 contains following 2 patches.
- 69e8f03c5ced3e4e6fb4181f4dac185104e3420b ("mm: backing-dev: Take a
reference to the bdi in use to prevent UAF")
- 80d91b86a199798ee2321a0ab0f09e6e12764678 ("fs: explicitly unregister
per-superblock BDIs")

The first commit 69e8f03("mm: backing-dev: Take a reference to the bdi
in use to prevent UAF") is not merged in the mainline and stable
kernels.
Commit 80d91b8 was merged in 5.16-rc1(commit hash is
0b3ea0926afb8dde70cfab00316ae0a70b93a7cc) which requires commit
c6fd3ac ("mm: export bdi_unregister") that exports symbol of
bdi_unregister().

Fixed status
mainline: [0b3ea0926afb8dde70cfab00316ae0a70b93a7cc]
---

As mentioned above, patch 69e8f03c5ced ("mm: backing-dev: Take a
reference to the bdi in use to prevent UAF") in asop 4.14 can fix
a null dereference problem, form my analysis, may like below:

blk_cleanup_queue
     blk_put_queue
         kobject_put(&q->kobj)
                 blk_release_queue
                     blk_exit_queue
                         bdi_put
                             release_bdi  // bdi -> null
del_gendisk
     bdi_unregister(disk->queue->backing_dev_info) // null -> reference

 From my analysis, In asop 4.14 kernel, in loop_remove function, there is
such a timing that executing "blk_cleanup_queue" first and then 
"del_gendisk".
but because of the refcnt of queue will add by hte line
"WARN_ON_ONCE(!blk_get_queue(disk->queue));" in "device_add_disk", which may
not result in "bdi_put" releasing bdi  in "blk_cleanup_queue".

I'm not sure where the problem is, so I want to ask Lee Jones who sent 
this patch
for clarification.

Thanks!

Wensheng

在 2022/8/12 13:44, Christoph Hellwig 写道:
> On Fri, Aug 12, 2022 at 11:34:59AM +0800, zhangwensheng (E) wrote:
>> Hi Lee ：
>>      I saw your patch because of CVE-2022-20158, the patch like below:
>>
>> ---
>>      mm: backing-dev: Take a reference to the bdi in use to prevent UAF
> I can't see that patch anywhere, and I've not seen an bug report for it.
>
>>      Because of a distinct lack of locking and/or reference taking,
>>      blk_cleanup_queue() puts the final taken reference to the bdi, which
> .. and blk_cleanup_queue also is gone upstream.
>
> What am I missing?
> .

Re: Question: consult patch

[Lee Jones]

On Fri, 12 Aug 2022, zhangwensheng (E) wrote:

> Hi
> 
> In CVE list last week, there is a new cve reported in asop 4.14 like below:
> Reference link: https://lore.kernel.org/all/CAODzB9rgMexvLjE=WuTm+SN8SfUggaZgWG-aBcy6cotppju6mw@mail.gmail.com/T/
> 
> ---
> CVE-2022-20158: mm: backing-dev: Take a reference to the bdi in use to
> prevent UAF
> 
> CVSS v3 score is not assigned.
> 
> AOSP kernel 4.14 contains following 2 patches.
> - 69e8f03c5ced3e4e6fb4181f4dac185104e3420b ("mm: backing-dev: Take a
> reference to the bdi in use to prevent UAF")
> - 80d91b86a199798ee2321a0ab0f09e6e12764678 ("fs: explicitly unregister
> per-superblock BDIs")
> 
> The first commit 69e8f03("mm: backing-dev: Take a reference to the bdi
> in use to prevent UAF") is not merged in the mainline and stable
> kernels.
> Commit 80d91b8 was merged in 5.16-rc1(commit hash is
> 0b3ea0926afb8dde70cfab00316ae0a70b93a7cc) which requires commit
> c6fd3ac ("mm: export bdi_unregister") that exports symbol of
> bdi_unregister().
> 
> Fixed status
> mainline: [0b3ea0926afb8dde70cfab00316ae0a70b93a7cc]
> ---
> 
> As mentioned above, patch 69e8f03c5ced ("mm: backing-dev: Take a
> reference to the bdi in use to prevent UAF") in asop 4.14 can fix
> a null dereference problem, form my analysis, may like below:
> 
> blk_cleanup_queue
>     blk_put_queue
>         kobject_put(&q->kobj)
>                 blk_release_queue
>                     blk_exit_queue
>                         bdi_put
>                             release_bdi  // bdi -> null
> del_gendisk
>     bdi_unregister(disk->queue->backing_dev_info) // null -> reference
> 
> From my analysis, In asop 4.14 kernel, in loop_remove function, there is
> such a timing that executing "blk_cleanup_queue" first and then
> "del_gendisk".
> but because of the refcnt of queue will add by hte line
> "WARN_ON_ONCE(!blk_get_queue(disk->queue));" in "device_add_disk", which may
> not result in "bdi_put" releasing bdi  in "blk_cleanup_queue".
> 
> I'm not sure where the problem is, so I want to ask Lee Jones who sent this
> patch
> for clarification.
> 
> Thanks!
> 
> Wensheng
> 
> 在 2022/8/12 13:44, Christoph Hellwig 写道:
> > On Fri, Aug 12, 2022 at 11:34:59AM +0800, zhangwensheng (E) wrote:
> > > Hi Lee ：
> > >      I saw your patch because of CVE-2022-20158, the patch like below:
> > > 
> > > ---
> > >      mm: backing-dev: Take a reference to the bdi in use to prevent UAF
> > I can't see that patch anywhere, and I've not seen an bug report for it.
> > 
> > >      Because of a distinct lack of locking and/or reference taking,
> > >      blk_cleanup_queue() puts the final taken reference to the bdi, which
> > .. and blk_cleanup_queue also is gone upstream.
> > 
> > What am I missing?

The issue reported in the aforementioned CVE was caused by a commit
which was applied to an internal, device specific repository.  One
that has never existed in Mainline.  I failed to reproduce the KASAN
report in any upstream or stable tree without the offending patch
applied.  The issue was fixed in all affected internal trees.

-- 
DEPRECATED: Please use lee@kernel.org

Re: Question: consult patch

[zhangwensheng (E)]

Hi Lee

I'm sorry I didn't fully understand you. did you mean the CVE
was caused by a commit which was applied to an internal kernel
version?  What commitment can be displayed? and I know that
mainline do not have the problem because of some code refactoring,
But are there any problems with some other stable branches?

thank you very much!
Wensheng

在 2022/8/12 15:26, Lee Jones 写道:
> On Fri, 12 Aug 2022, zhangwensheng (E) wrote:
>
>> Hi
>>
>> In CVE list last week, there is a new cve reported in asop 4.14 like below:
>> Reference link: https://lore.kernel.org/all/CAODzB9rgMexvLjE=WuTm+SN8SfUggaZgWG-aBcy6cotppju6mw@mail.gmail.com/T/
>>
>> ---
>> CVE-2022-20158: mm: backing-dev: Take a reference to the bdi in use to
>> prevent UAF
>>
>> CVSS v3 score is not assigned.
>>
>> AOSP kernel 4.14 contains following 2 patches.
>> - 69e8f03c5ced3e4e6fb4181f4dac185104e3420b ("mm: backing-dev: Take a
>> reference to the bdi in use to prevent UAF")
>> - 80d91b86a199798ee2321a0ab0f09e6e12764678 ("fs: explicitly unregister
>> per-superblock BDIs")
>>
>> The first commit 69e8f03("mm: backing-dev: Take a reference to the bdi
>> in use to prevent UAF") is not merged in the mainline and stable
>> kernels.
>> Commit 80d91b8 was merged in 5.16-rc1(commit hash is
>> 0b3ea0926afb8dde70cfab00316ae0a70b93a7cc) which requires commit
>> c6fd3ac ("mm: export bdi_unregister") that exports symbol of
>> bdi_unregister().
>>
>> Fixed status
>> mainline: [0b3ea0926afb8dde70cfab00316ae0a70b93a7cc]
>> ---
>>
>> As mentioned above, patch 69e8f03c5ced ("mm: backing-dev: Take a
>> reference to the bdi in use to prevent UAF") in asop 4.14 can fix
>> a null dereference problem, form my analysis, may like below:
>>
>> blk_cleanup_queue
>>      blk_put_queue
>>          kobject_put(&q->kobj)
>>                  blk_release_queue
>>                      blk_exit_queue
>>                          bdi_put
>>                              release_bdi  // bdi -> null
>> del_gendisk
>>      bdi_unregister(disk->queue->backing_dev_info) // null -> reference
>>
>>  From my analysis, In asop 4.14 kernel, in loop_remove function, there is
>> such a timing that executing "blk_cleanup_queue" first and then
>> "del_gendisk".
>> but because of the refcnt of queue will add by hte line
>> "WARN_ON_ONCE(!blk_get_queue(disk->queue));" in "device_add_disk", which may
>> not result in "bdi_put" releasing bdi  in "blk_cleanup_queue".
>>
>> I'm not sure where the problem is, so I want to ask Lee Jones who sent this
>> patch
>> for clarification.
>>
>> Thanks!
>>
>> Wensheng
>>
>> 在 2022/8/12 13:44, Christoph Hellwig 写道:
>>> On Fri, Aug 12, 2022 at 11:34:59AM +0800, zhangwensheng (E) wrote:
>>>> Hi Lee ：
>>>>       I saw your patch because of CVE-2022-20158, the patch like below:
>>>>
>>>> ---
>>>>       mm: backing-dev: Take a reference to the bdi in use to prevent UAF
>>> I can't see that patch anywhere, and I've not seen an bug report for it.
>>>
>>>>       Because of a distinct lack of locking and/or reference taking,
>>>>       blk_cleanup_queue() puts the final taken reference to the bdi, which
>>> .. and blk_cleanup_queue also is gone upstream.
>>>
>>> What am I missing?
> The issue reported in the aforementioned CVE was caused by a commit
> which was applied to an internal, device specific repository.  One
> that has never existed in Mainline.  I failed to reproduce the KASAN
> report in any upstream or stable tree without the offending patch
> applied.  The issue was fixed in all affected internal trees.
>

Re: Question: consult patch

[Lee Jones]

> thank you very much!

Please refrain from top-posting.

> 在 2022/8/12 15:26, Lee Jones 写道:
> > On Fri, 12 Aug 2022, zhangwensheng (E) wrote:
> > 
> > > Hi
> > > 
> > > In CVE list last week, there is a new cve reported in asop 4.14 like below:
> > > Reference link: https://lore.kernel.org/all/CAODzB9rgMexvLjE=WuTm+SN8SfUggaZgWG-aBcy6cotppju6mw@mail.gmail.com/T/
> > > 
> > > ---
> > > CVE-2022-20158: mm: backing-dev: Take a reference to the bdi in use to
> > > prevent UAF
> > > 
> > > CVSS v3 score is not assigned.
> > > 
> > > AOSP kernel 4.14 contains following 2 patches.
> > > - 69e8f03c5ced3e4e6fb4181f4dac185104e3420b ("mm: backing-dev: Take a
> > > reference to the bdi in use to prevent UAF")
> > > - 80d91b86a199798ee2321a0ab0f09e6e12764678 ("fs: explicitly unregister
> > > per-superblock BDIs")
> > > 
> > > The first commit 69e8f03("mm: backing-dev: Take a reference to the bdi
> > > in use to prevent UAF") is not merged in the mainline and stable
> > > kernels.
> > > Commit 80d91b8 was merged in 5.16-rc1(commit hash is
> > > 0b3ea0926afb8dde70cfab00316ae0a70b93a7cc) which requires commit
> > > c6fd3ac ("mm: export bdi_unregister") that exports symbol of
> > > bdi_unregister().
> > > 
> > > Fixed status
> > > mainline: [0b3ea0926afb8dde70cfab00316ae0a70b93a7cc]
> > > ---
> > > 
> > > As mentioned above, patch 69e8f03c5ced ("mm: backing-dev: Take a
> > > reference to the bdi in use to prevent UAF") in asop 4.14 can fix
> > > a null dereference problem, form my analysis, may like below:
> > > 
> > > blk_cleanup_queue
> > >      blk_put_queue
> > >          kobject_put(&q->kobj)
> > >                  blk_release_queue
> > >                      blk_exit_queue
> > >                          bdi_put
> > >                              release_bdi  // bdi -> null
> > > del_gendisk
> > >      bdi_unregister(disk->queue->backing_dev_info) // null -> reference
> > > 
> > >  From my analysis, In asop 4.14 kernel, in loop_remove function, there is
> > > such a timing that executing "blk_cleanup_queue" first and then
> > > "del_gendisk".
> > > but because of the refcnt of queue will add by hte line
> > > "WARN_ON_ONCE(!blk_get_queue(disk->queue));" in "device_add_disk", which may
> > > not result in "bdi_put" releasing bdi  in "blk_cleanup_queue".
> > > 
> > > I'm not sure where the problem is, so I want to ask Lee Jones who sent this
> > > patch
> > > for clarification.
> > > 
> > > Thanks!
> > > 
> > > Wensheng
> > > 
> > > 在 2022/8/12 13:44, Christoph Hellwig 写道:
> > > > On Fri, Aug 12, 2022 at 11:34:59AM +0800, zhangwensheng (E) wrote:
> > > > > Hi Lee ：
> > > > >       I saw your patch because of CVE-2022-20158, the patch like below:
> > > > > 
> > > > > ---
> > > > >       mm: backing-dev: Take a reference to the bdi in use to prevent UAF
> > > > I can't see that patch anywhere, and I've not seen an bug report for it.
> > > > 
> > > > >       Because of a distinct lack of locking and/or reference taking,
> > > > >       blk_cleanup_queue() puts the final taken reference to the bdi, which
> > > > .. and blk_cleanup_queue also is gone upstream.
> > > > 
> > > > What am I missing?
> > The issue reported in the aforementioned CVE was caused by a commit
> > which was applied to an internal, device specific repository.  One
> > that has never existed in Mainline.  I failed to reproduce the KASAN
> > report in any upstream or stable tree without the offending patch
> > applied.  The issue was fixed in all affected internal trees.
> 
> I'm sorry I didn't fully understand you. did you mean the CVE
> was caused by a commit which was applied to an internal kernel
> version?  What commitment can be displayed? and I know that
> mainline do not have the problem because of some code refactoring,
> But are there any problems with some other stable branches?

Yes, CVE-2022-20158 pertains to an issue on an internal repo.

Mainline and Stable were unaffected.

-- 
DEPRECATED: Please use lee@kernel.org

Re: Question: consult patch

[zhangwensheng (E)]

thanks for your reply!

在 2022/8/12 15:38, zhangwensheng (E) 写道:
> Hi Lee
>
> I'm sorry I didn't fully understand you. did you mean the CVE
> was caused by a commit which was applied to an internal kernel
> version?  What commitment can be displayed? and I know that
> mainline do not have the problem because of some code refactoring,
> But are there any problems with some other stable branches?
>
> thank you very much!
> Wensheng
>
> 在 2022/8/12 15:26, Lee Jones 写道:
>> On Fri, 12 Aug 2022, zhangwensheng (E) wrote:
>>
>>> Hi
>>>
>>> In CVE list last week, there is a new cve reported in asop 4.14 like 
>>> below:
>>> Reference link: 
>>> https://lore.kernel.org/all/CAODzB9rgMexvLjE=WuTm+SN8SfUggaZgWG-aBcy6cotppju6mw@mail.gmail.com/T/
>>>
>>> ---
>>> CVE-2022-20158: mm: backing-dev: Take a reference to the bdi in use to
>>> prevent UAF
>>>
>>> CVSS v3 score is not assigned.
>>>
>>> AOSP kernel 4.14 contains following 2 patches.
>>> - 69e8f03c5ced3e4e6fb4181f4dac185104e3420b ("mm: backing-dev: Take a
>>> reference to the bdi in use to prevent UAF")
>>> - 80d91b86a199798ee2321a0ab0f09e6e12764678 ("fs: explicitly unregister
>>> per-superblock BDIs")
>>>
>>> The first commit 69e8f03("mm: backing-dev: Take a reference to the bdi
>>> in use to prevent UAF") is not merged in the mainline and stable
>>> kernels.
>>> Commit 80d91b8 was merged in 5.16-rc1(commit hash is
>>> 0b3ea0926afb8dde70cfab00316ae0a70b93a7cc) which requires commit
>>> c6fd3ac ("mm: export bdi_unregister") that exports symbol of
>>> bdi_unregister().
>>>
>>> Fixed status
>>> mainline: [0b3ea0926afb8dde70cfab00316ae0a70b93a7cc]
>>> ---
>>>
>>> As mentioned above, patch 69e8f03c5ced ("mm: backing-dev: Take a
>>> reference to the bdi in use to prevent UAF") in asop 4.14 can fix
>>> a null dereference problem, form my analysis, may like below:
>>>
>>> blk_cleanup_queue
>>>      blk_put_queue
>>>          kobject_put(&q->kobj)
>>>                  blk_release_queue
>>>                      blk_exit_queue
>>>                          bdi_put
>>>                              release_bdi  // bdi -> null
>>> del_gendisk
>>>      bdi_unregister(disk->queue->backing_dev_info) // null -> reference
>>>
>>>  From my analysis, In asop 4.14 kernel, in loop_remove function, 
>>> there is
>>> such a timing that executing "blk_cleanup_queue" first and then
>>> "del_gendisk".
>>> but because of the refcnt of queue will add by hte line
>>> "WARN_ON_ONCE(!blk_get_queue(disk->queue));" in "device_add_disk", 
>>> which may
>>> not result in "bdi_put" releasing bdi  in "blk_cleanup_queue".
>>>
>>> I'm not sure where the problem is, so I want to ask Lee Jones who 
>>> sent this
>>> patch
>>> for clarification.
>>>
>>> Thanks!
>>>
>>> Wensheng
>>>
>>> 在 2022/8/12 13:44, Christoph Hellwig 写道:
>>>> On Fri, Aug 12, 2022 at 11:34:59AM +0800, zhangwensheng (E) wrote:
>>>>> Hi Lee ：
>>>>>       I saw your patch because of CVE-2022-20158, the patch like 
>>>>> below:
>>>>>
>>>>> ---
>>>>>       mm: backing-dev: Take a reference to the bdi in use to 
>>>>> prevent UAF
>>>> I can't see that patch anywhere, and I've not seen an bug report 
>>>> for it.
>>>>
>>>>>       Because of a distinct lack of locking and/or reference taking,
>>>>>       blk_cleanup_queue() puts the final taken reference to the 
>>>>> bdi, which
>>>> .. and blk_cleanup_queue also is gone upstream.
>>>>
>>>> What am I missing?
>> The issue reported in the aforementioned CVE was caused by a commit
>> which was applied to an internal, device specific repository. One
>> that has never existed in Mainline.  I failed to reproduce the KASAN
>> report in any upstream or stable tree without the offending patch
>> applied.  The issue was fixed in all affected internal trees.
>>
>
> .