| Message ID | 20230606033212.1068823-3-xiubli@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: fix fscrypt_destroy_keyring use-after-free bug | expand |
Looks good to me. Reviewed-by: Milind Changire <mchangir@redhat.com> On Tue, Jun 6, 2023 at 9:04 AM <xiubli@redhat.com> wrote: > > From: Xiubo Li <xiubli@redhat.com> > > The sync_filesystem() will flush all the dirty buffer and submit the > osd reqs to the osdc and then is blocked to wait for all the reqs to > finish. But the when the reqs' replies come, the reqs will be removed > from osdc just before the req->r_callback()s are called. Which means > the sync_filesystem() will be woke up by leaving the req->r_callback()s > are still running. > > This will be buggy when the waiter require the req->r_callback()s to > release some resources before continuing. So we need to make sure the > req->r_callback()s are called before removing the reqs from the osdc. > > WARNING: CPU: 4 PID: 168846 at fs/crypto/keyring.c:242 fscrypt_destroy_keyring+0x7e/0xd0 > CPU: 4 PID: 168846 Comm: umount Tainted: G S 6.1.0-rc5-ceph-g72ead199864c #1 > Hardware name: Supermicro SYS-5018R-WR/X10SRW-F, BIOS 2.0 12/17/2015 > RIP: 0010:fscrypt_destroy_keyring+0x7e/0xd0 > RSP: 0018:ffffc9000b277e28 EFLAGS: 00010202 > RAX: 0000000000000002 RBX: ffff88810d52ac00 RCX: ffff88810b56aa00 > RDX: 0000000080000000 RSI: ffffffff822f3a09 RDI: ffff888108f59000 > RBP: ffff8881d394fb88 R08: 0000000000000028 R09: 0000000000000000 > R10: 0000000000000001 R11: 11ff4fe6834fcd91 R12: ffff8881d394fc40 > R13: ffff888108f59000 R14: ffff8881d394f800 R15: 0000000000000000 > FS: 00007fd83f6f1080(0000) GS:ffff88885fd00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007f918d417000 CR3: 000000017f89a005 CR4: 00000000003706e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > <TASK> > generic_shutdown_super+0x47/0x120 > kill_anon_super+0x14/0x30 > ceph_kill_sb+0x36/0x90 [ceph] > deactivate_locked_super+0x29/0x60 > cleanup_mnt+0xb8/0x140 > task_work_run+0x67/0xb0 > exit_to_user_mode_prepare+0x23d/0x240 > syscall_exit_to_user_mode+0x25/0x60 > do_syscall_64+0x40/0x80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > RIP: 0033:0x7fd83dc39e9b > > We need to increase the blocker counter to make sure all the osd > requests' callbacks have been finished just before calling the > kill_anon_super() when unmounting. > > URL: https://tracker.ceph.com/issues/58126 > Signed-off-by: Xiubo Li <xiubli@redhat.com> > --- > fs/ceph/addr.c | 10 ++++++++++ > fs/ceph/super.c | 11 +++++++++++ > fs/ceph/super.h | 2 ++ > 3 files changed, 23 insertions(+) > > diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c > index 78ad45567dbb..de9b82905f18 100644 > --- a/fs/ceph/addr.c > +++ b/fs/ceph/addr.c > @@ -284,6 +284,7 @@ static void finish_netfs_read(struct ceph_osd_request *req) > } > netfs_subreq_terminated(subreq, err, false); > iput(req->r_inode); > + ceph_dec_osd_stopping_blocker(fsc->mdsc); > } > > static bool ceph_netfs_issue_op_inline(struct netfs_io_subrequest *subreq) > @@ -411,6 +412,10 @@ static void ceph_netfs_issue_read(struct netfs_io_subrequest *subreq) > } else { > osd_req_op_extent_osd_iter(req, 0, &iter); > } > + if (!ceph_inc_osd_stopping_blocker(fsc->mdsc)) { > + err = -EIO; > + goto out; > + } > req->r_callback = finish_netfs_read; > req->r_priv = subreq; > req->r_inode = inode; > @@ -906,6 +911,7 @@ static void writepages_finish(struct ceph_osd_request *req) > else > kfree(osd_data->pages); > ceph_osdc_put_request(req); > + ceph_dec_osd_stopping_blocker(fsc->mdsc); > } > > /* > @@ -1214,6 +1220,10 @@ static int ceph_writepages_start(struct address_space *mapping, > BUG_ON(len < ceph_fscrypt_page_offset(pages[locked_pages - 1]) + > thp_size(pages[locked_pages - 1]) - offset); > > + if (!ceph_inc_osd_stopping_blocker(fsc->mdsc)) { > + rc = -EIO; > + goto release_folios; > + } > req->r_callback = writepages_finish; > req->r_inode = inode; > > diff --git a/fs/ceph/super.c b/fs/ceph/super.c > index d3f54f3d7b17..401fe61ea53a 100644 > --- a/fs/ceph/super.c > +++ b/fs/ceph/super.c > @@ -1524,6 +1524,17 @@ void ceph_dec_mds_stopping_blocker(struct ceph_mds_client *mdsc) > __dec_stopping_blocker(mdsc); > } > > +/* For data IO requests */ > +bool ceph_inc_osd_stopping_blocker(struct ceph_mds_client *mdsc) > +{ > + return __inc_stopping_blocker(mdsc); > +} > + > +void ceph_dec_osd_stopping_blocker(struct ceph_mds_client *mdsc) > +{ > + __dec_stopping_blocker(mdsc); > +} > + > static void ceph_kill_sb(struct super_block *s) > { > struct ceph_fs_client *fsc = ceph_sb_to_client(s); > diff --git a/fs/ceph/super.h b/fs/ceph/super.h > index cd5b88d819ca..2f9b6fc667b8 100644 > --- a/fs/ceph/super.h > +++ b/fs/ceph/super.h > @@ -1418,4 +1418,6 @@ extern void ceph_cleanup_quotarealms_inodes(struct ceph_mds_client *mdsc); > bool ceph_inc_mds_stopping_blocker(struct ceph_mds_client *mdsc, > struct ceph_mds_session *session); > void ceph_dec_mds_stopping_blocker(struct ceph_mds_client *mdsc); > +bool ceph_inc_osd_stopping_blocker(struct ceph_mds_client *mdsc); > +void ceph_dec_osd_stopping_blocker(struct ceph_mds_client *mdsc); > #endif /* _FS_CEPH_SUPER_H */ > -- > 2.40.1 >
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c index 78ad45567dbb..de9b82905f18 100644 --- a/fs/ceph/addr.c +++ b/fs/ceph/addr.c @@ -284,6 +284,7 @@ static void finish_netfs_read(struct ceph_osd_request *req) } netfs_subreq_terminated(subreq, err, false); iput(req->r_inode); + ceph_dec_osd_stopping_blocker(fsc->mdsc); } static bool ceph_netfs_issue_op_inline(struct netfs_io_subrequest *subreq) @@ -411,6 +412,10 @@ static void ceph_netfs_issue_read(struct netfs_io_subrequest *subreq) } else { osd_req_op_extent_osd_iter(req, 0, &iter); } + if (!ceph_inc_osd_stopping_blocker(fsc->mdsc)) { + err = -EIO; + goto out; + } req->r_callback = finish_netfs_read; req->r_priv = subreq; req->r_inode = inode; @@ -906,6 +911,7 @@ static void writepages_finish(struct ceph_osd_request *req) else kfree(osd_data->pages); ceph_osdc_put_request(req); + ceph_dec_osd_stopping_blocker(fsc->mdsc); } /* @@ -1214,6 +1220,10 @@ static int ceph_writepages_start(struct address_space *mapping, BUG_ON(len < ceph_fscrypt_page_offset(pages[locked_pages - 1]) + thp_size(pages[locked_pages - 1]) - offset); + if (!ceph_inc_osd_stopping_blocker(fsc->mdsc)) { + rc = -EIO; + goto release_folios; + } req->r_callback = writepages_finish; req->r_inode = inode; diff --git a/fs/ceph/super.c b/fs/ceph/super.c index d3f54f3d7b17..401fe61ea53a 100644 --- a/fs/ceph/super.c +++ b/fs/ceph/super.c @@ -1524,6 +1524,17 @@ void ceph_dec_mds_stopping_blocker(struct ceph_mds_client *mdsc) __dec_stopping_blocker(mdsc); } +/* For data IO requests */ +bool ceph_inc_osd_stopping_blocker(struct ceph_mds_client *mdsc) +{ + return __inc_stopping_blocker(mdsc); +} + +void ceph_dec_osd_stopping_blocker(struct ceph_mds_client *mdsc) +{ + __dec_stopping_blocker(mdsc); +} + static void ceph_kill_sb(struct super_block *s) { struct ceph_fs_client *fsc = ceph_sb_to_client(s); diff --git a/fs/ceph/super.h b/fs/ceph/super.h index cd5b88d819ca..2f9b6fc667b8 100644 --- a/fs/ceph/super.h +++ b/fs/ceph/super.h @@ -1418,4 +1418,6 @@ extern void ceph_cleanup_quotarealms_inodes(struct ceph_mds_client *mdsc); bool ceph_inc_mds_stopping_blocker(struct ceph_mds_client *mdsc, struct ceph_mds_session *session); void ceph_dec_mds_stopping_blocker(struct ceph_mds_client *mdsc); +bool ceph_inc_osd_stopping_blocker(struct ceph_mds_client *mdsc); +void ceph_dec_osd_stopping_blocker(struct ceph_mds_client *mdsc); #endif /* _FS_CEPH_SUPER_H */