mbox series

[v3,00/12] cachefiles: some bugfixes and cleanups for ondemand requests

Message ID 20240522114308.2402121-1-libaokun@huaweicloud.com (mailing list archive)
Headers show
Series cachefiles: some bugfixes and cleanups for ondemand requests | expand

Message

Baokun Li May 22, 2024, 11:42 a.m. UTC
From: Baokun Li <libaokun1@huawei.com>

Hi all!

This is the third version of this patch series. The new version has no
functional changes compared to the previous one, so I've kept the previous
Acked-by and Reviewed-by, so please let me know if you have any objections.

Thank you, Jia Zhu and Jingbo Xu, Jeff Layton, Gao Xiang, for the feedback
in the previous version.

We've been testing ondemand mode for cachefiles since January, and we're
almost done. We hit a lot of issues during the testing period, and this
patch set fixes some of the issues related to ondemand requests.
The patches have passed internal testing without regression.

The following is a brief overview of the patches, see the patches for
more details.

Patch 1-5: Holding reference counts of reqs and objects on read requests
to avoid malicious restore leading to use-after-free.

Patch 6-10: Add some consistency checks to copen/cread/get_fd to avoid
malicious copen/cread/close fd injections causing use-after-free or hung.

Patch 11: When cache is marked as CACHEFILES_DEAD, flush all requests,
otherwise the kernel may be hung. since this state is irreversible, the
daemon can read open requests but cannot copen.

Patch 12: Allow interrupting a read request being processed by killing
the read process as a way of avoiding hung in some special cases.

Comments and questions are, as always, welcome.
Please let me know what you think.

Thanks,
Baokun

Changes since v2:
  * Collect Acked-by from Jeff Layton.(Thanks for your ack!)
  * Collect RVB from Gao Xiang and Jingbo Xu.(Thanks for your review!)
  * Pathch 9: Rename anon_file to ondemand_anon_file to avoid possible
    conflicts with generic code.
  * Pathch 12: Add cachefiles_ondemand_finish_req() helper function to
    simplify the code.
  * Adjust the patch order as suggested to facilitate backporting to
    the STABLE version.
    * The current patch 1 is the previous patch 5;
    * The current patch 5 is the previous patch 2;

Changes since v1:
  * Collect RVB from Jia Zhu and Jingbo Xu.(Thanks for your review!)
  * Pathch 1: Add Fixes tag and enrich the commit message.
  * Pathch 7: Add function graph comments.
  * Pathch 8: Update commit message and comments.
  * Pathch 9: Enriched commit msg.

[V1]: https://lore.kernel.org/all/20240424033916.2748488-1-libaokun@huaweicloud.com
[V2]: https://lore.kernel.org/all/20240515084601.3240503-1-libaokun@huaweicloud.com


Baokun Li (11):
  cachefiles: add output string to cachefiles_obj_[get|put]_ondemand_fd
  cachefiles: remove requests from xarray during flushing requests
  cachefiles: fix slab-use-after-free in cachefiles_ondemand_get_fd()
  cachefiles: fix slab-use-after-free in
    cachefiles_ondemand_daemon_read()
  cachefiles: remove err_put_fd label in
    cachefiles_ondemand_daemon_read()
  cachefiles: add consistency check for copen/cread
  cachefiles: add spin_lock for cachefiles_ondemand_info
  cachefiles: never get a new anonymous fd if ondemand_id is valid
  cachefiles: defer exposing anon_fd until after copy_to_user() succeeds
  cachefiles: flush all requests after setting CACHEFILES_DEAD
  cachefiles: make on-demand read killable

Zizhi Wo (1):
  cachefiles: Set object to close if ondemand_id < 0 in copen

 fs/cachefiles/daemon.c            |   3 +-
 fs/cachefiles/internal.h          |   5 +
 fs/cachefiles/ondemand.c          | 217 ++++++++++++++++++++++--------
 include/trace/events/cachefiles.h |   8 +-
 4 files changed, 176 insertions(+), 57 deletions(-)

Comments

Christian Brauner May 29, 2024, 11:07 a.m. UTC | #1
On Wed, 22 May 2024 19:42:56 +0800, libaokun@huaweicloud.com wrote:
> From: Baokun Li <libaokun1@huawei.com>
> 
> Hi all!
> 
> This is the third version of this patch series. The new version has no
> functional changes compared to the previous one, so I've kept the previous
> Acked-by and Reviewed-by, so please let me know if you have any objections.
> 
> [...]

So I've taken that as a fixes series which should probably make it upstream
rather sooner than later. Correct?

---

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[01/12] cachefiles: add output string to cachefiles_obj_[get|put]_ondemand_fd
        https://git.kernel.org/vfs/vfs/c/cc5ac966f261
[02/12] cachefiles: remove requests from xarray during flushing requests
        https://git.kernel.org/vfs/vfs/c/0fc75c5940fa
[03/12] cachefiles: fix slab-use-after-free in cachefiles_ondemand_get_fd()
        https://git.kernel.org/vfs/vfs/c/de3e26f9e5b7
[04/12] cachefiles: fix slab-use-after-free in cachefiles_ondemand_daemon_read()
        https://git.kernel.org/vfs/vfs/c/da4a82741606
[05/12] cachefiles: remove err_put_fd label in cachefiles_ondemand_daemon_read()
        https://git.kernel.org/vfs/vfs/c/3e6d704f02aa
[06/12] cachefiles: add consistency check for copen/cread
        https://git.kernel.org/vfs/vfs/c/a26dc49df37e
[07/12] cachefiles: add spin_lock for cachefiles_ondemand_info
        https://git.kernel.org/vfs/vfs/c/0a790040838c
[08/12] cachefiles: never get a new anonymous fd if ondemand_id is valid
        https://git.kernel.org/vfs/vfs/c/4988e35e95fc
[09/12] cachefiles: defer exposing anon_fd until after copy_to_user() succeeds
        https://git.kernel.org/vfs/vfs/c/4b4391e77a6b
[10/12] cachefiles: Set object to close if ondemand_id < 0 in copen
        https://git.kernel.org/vfs/vfs/c/4f8703fb3482
[11/12] cachefiles: flush all requests after setting CACHEFILES_DEAD
        https://git.kernel.org/vfs/vfs/c/85e833cd7243
[12/12] cachefiles: make on-demand read killable
        https://git.kernel.org/vfs/vfs/c/bc9dde615546
Baokun Li May 29, 2024, 11:23 a.m. UTC | #2
On 2024/5/29 19:07, Christian Brauner wrote:
> On Wed, 22 May 2024 19:42:56 +0800, libaokun@huaweicloud.com wrote:
>> From: Baokun Li <libaokun1@huawei.com>
>>
>> Hi all!
>>
>> This is the third version of this patch series. The new version has no
>> functional changes compared to the previous one, so I've kept the previous
>> Acked-by and Reviewed-by, so please let me know if you have any objections.
>>
>> [...]
> So I've taken that as a fixes series which should probably make it upstream
> rather sooner than later. Correct?
Yes, I hope this patch set could be upstream soon.
Thank you very much for applying this fixes series!

Regards,
Baokun
> ---
>
> Applied to the vfs.fixes branch of the vfs/vfs.git tree.
> Patches in the vfs.fixes branch should appear in linux-next soon.
>
> Please report any outstanding bugs that were missed during review in a
> new review to the original patch series allowing us to drop it.
>
> It's encouraged to provide Acked-bys and Reviewed-bys even though the
> patch has now been applied. If possible patch trailers will be updated.
>
> Note that commit hashes shown below are subject to change due to rebase,
> trailer updates or similar. If in doubt, please check the listed branch.
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
> branch: vfs.fixes
>
> [01/12] cachefiles: add output string to cachefiles_obj_[get|put]_ondemand_fd
>          https://git.kernel.org/vfs/vfs/c/cc5ac966f261
> [02/12] cachefiles: remove requests from xarray during flushing requests
>          https://git.kernel.org/vfs/vfs/c/0fc75c5940fa
> [03/12] cachefiles: fix slab-use-after-free in cachefiles_ondemand_get_fd()
>          https://git.kernel.org/vfs/vfs/c/de3e26f9e5b7
> [04/12] cachefiles: fix slab-use-after-free in cachefiles_ondemand_daemon_read()
>          https://git.kernel.org/vfs/vfs/c/da4a82741606
> [05/12] cachefiles: remove err_put_fd label in cachefiles_ondemand_daemon_read()
>          https://git.kernel.org/vfs/vfs/c/3e6d704f02aa
> [06/12] cachefiles: add consistency check for copen/cread
>          https://git.kernel.org/vfs/vfs/c/a26dc49df37e
> [07/12] cachefiles: add spin_lock for cachefiles_ondemand_info
>          https://git.kernel.org/vfs/vfs/c/0a790040838c
> [08/12] cachefiles: never get a new anonymous fd if ondemand_id is valid
>          https://git.kernel.org/vfs/vfs/c/4988e35e95fc
> [09/12] cachefiles: defer exposing anon_fd until after copy_to_user() succeeds
>          https://git.kernel.org/vfs/vfs/c/4b4391e77a6b
> [10/12] cachefiles: Set object to close if ondemand_id < 0 in copen
>          https://git.kernel.org/vfs/vfs/c/4f8703fb3482
> [11/12] cachefiles: flush all requests after setting CACHEFILES_DEAD
>          https://git.kernel.org/vfs/vfs/c/85e833cd7243
> [12/12] cachefiles: make on-demand read killable
>          https://git.kernel.org/vfs/vfs/c/bc9dde615546
Gao Xiang May 29, 2024, 2:28 p.m. UTC | #3
Hi Christian,

On 2024/5/29 19:07, Christian Brauner wrote:
> On Wed, 22 May 2024 19:42:56 +0800, libaokun@huaweicloud.com wrote:
>> From: Baokun Li <libaokun1@huawei.com>
>>
>> Hi all!
>>
>> This is the third version of this patch series. The new version has no
>> functional changes compared to the previous one, so I've kept the previous
>> Acked-by and Reviewed-by, so please let me know if you have any objections.
>>
>> [...]
> 
> So I've taken that as a fixes series which should probably make it upstream
> rather sooner than later. Correct?

Yeah, many thanks for picking these up!  AFAIK, they've already been
landed downstream for a while so it'd be much better to address
these upstream. :-)

Thanks,
Gao Xiang

> 
> ---
> 
> Applied to the vfs.fixes branch of the vfs/vfs.git tree.
> Patches in the vfs.fixes branch should appear in linux-next soon.
> 
> Please report any outstanding bugs that were missed during review in a
> new review to the original patch series allowing us to drop it.
> 
> It's encouraged to provide Acked-bys and Reviewed-bys even though the
> patch has now been applied. If possible patch trailers will be updated.
> 
> Note that commit hashes shown below are subject to change due to rebase,
> trailer updates or similar. If in doubt, please check the listed branch.
> 
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
> branch: vfs.fixes
> 
> [01/12] cachefiles: add output string to cachefiles_obj_[get|put]_ondemand_fd
>          https://git.kernel.org/vfs/vfs/c/cc5ac966f261
> [02/12] cachefiles: remove requests from xarray during flushing requests
>          https://git.kernel.org/vfs/vfs/c/0fc75c5940fa
> [03/12] cachefiles: fix slab-use-after-free in cachefiles_ondemand_get_fd()
>          https://git.kernel.org/vfs/vfs/c/de3e26f9e5b7
> [04/12] cachefiles: fix slab-use-after-free in cachefiles_ondemand_daemon_read()
>          https://git.kernel.org/vfs/vfs/c/da4a82741606
> [05/12] cachefiles: remove err_put_fd label in cachefiles_ondemand_daemon_read()
>          https://git.kernel.org/vfs/vfs/c/3e6d704f02aa
> [06/12] cachefiles: add consistency check for copen/cread
>          https://git.kernel.org/vfs/vfs/c/a26dc49df37e
> [07/12] cachefiles: add spin_lock for cachefiles_ondemand_info
>          https://git.kernel.org/vfs/vfs/c/0a790040838c
> [08/12] cachefiles: never get a new anonymous fd if ondemand_id is valid
>          https://git.kernel.org/vfs/vfs/c/4988e35e95fc
> [09/12] cachefiles: defer exposing anon_fd until after copy_to_user() succeeds
>          https://git.kernel.org/vfs/vfs/c/4b4391e77a6b
> [10/12] cachefiles: Set object to close if ondemand_id < 0 in copen
>          https://git.kernel.org/vfs/vfs/c/4f8703fb3482
> [11/12] cachefiles: flush all requests after setting CACHEFILES_DEAD
>          https://git.kernel.org/vfs/vfs/c/85e833cd7243
> [12/12] cachefiles: make on-demand read killable
>          https://git.kernel.org/vfs/vfs/c/bc9dde615546