Skip to content

Commit c4e28be

Browse files
Luis Henriquesgregkh
Luis Henriques
authored andcommitted
ceph: fix buffer free while holding i_ceph_lock in __ceph_build_xattrs_blob()
[ Upstream commit 12fe3dd ] Calling ceph_buffer_put() in __ceph_build_xattrs_blob() may result in freeing the i_xattrs.blob buffer while holding the i_ceph_lock. This can be fixed by having this function returning the old blob buffer and have the callers of this function freeing it when the lock is released. The following backtrace was triggered by fstests generic/117. BUG: sleeping function called from invalid context at mm/vmalloc.c:2283 in_atomic(): 1, irqs_disabled(): 0, pid: 649, name: fsstress 4 locks held by fsstress/649: #0: 00000000a7478e7e (&type->s_umount_key#19){++++}, at: iterate_supers+0x77/0xf0 #1: 00000000f8de1423 (&(&ci->i_ceph_lock)->rlock){+.+.}, at: ceph_check_caps+0x7b/0xc60 #2: 00000000562f2b27 (&s->s_mutex){+.+.}, at: ceph_check_caps+0x3bd/0xc60 #3: 00000000f83ce16a (&mdsc->snap_rwsem){++++}, at: ceph_check_caps+0x3ed/0xc60 CPU: 1 PID: 649 Comm: fsstress Not tainted 5.2.0+ #439 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58-prebuilt.qemu.org 04/01/2014 Call Trace: dump_stack+0x67/0x90 ___might_sleep.cold+0x9f/0xb1 vfree+0x4b/0x60 ceph_buffer_release+0x1b/0x60 __ceph_build_xattrs_blob+0x12b/0x170 __send_cap+0x302/0x540 ? __lock_acquire+0x23c/0x1e40 ? __mark_caps_flushing+0x15c/0x280 ? _raw_spin_unlock+0x24/0x30 ceph_check_caps+0x5f0/0xc60 ceph_flush_dirty_caps+0x7c/0x150 ? __ia32_sys_fdatasync+0x20/0x20 ceph_sync_fs+0x5a/0x130 iterate_supers+0x8f/0xf0 ksys_sync+0x4f/0xb0 __ia32_sys_sync+0xa/0x10 do_syscall_64+0x50/0x1c0 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x7fc6409ab617 Signed-off-by: Luis Henriques <[email protected]> Reviewed-by: Jeff Layton <[email protected]> Signed-off-by: Ilya Dryomov <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent f41cd55 commit c4e28be

File tree

4 files changed

+16
-6
lines changed

4 files changed

+16
-6
lines changed

fs/ceph/caps.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1305,6 +1305,7 @@ static int __send_cap(struct ceph_mds_client *mdsc, struct ceph_cap *cap,
13051305
{
13061306
struct ceph_inode_info *ci = cap->ci;
13071307
struct inode *inode = &ci->vfs_inode;
1308+
struct ceph_buffer *old_blob = NULL;
13081309
struct cap_msg_args arg;
13091310
int held, revoking;
13101311
int wake = 0;
@@ -1369,7 +1370,7 @@ static int __send_cap(struct ceph_mds_client *mdsc, struct ceph_cap *cap,
13691370
ci->i_requested_max_size = arg.max_size;
13701371

13711372
if (flushing & CEPH_CAP_XATTR_EXCL) {
1372-
__ceph_build_xattrs_blob(ci);
1373+
old_blob = __ceph_build_xattrs_blob(ci);
13731374
arg.xattr_version = ci->i_xattrs.version;
13741375
arg.xattr_buf = ci->i_xattrs.blob;
13751376
} else {
@@ -1404,6 +1405,8 @@ static int __send_cap(struct ceph_mds_client *mdsc, struct ceph_cap *cap,
14041405

14051406
spin_unlock(&ci->i_ceph_lock);
14061407

1408+
ceph_buffer_put(old_blob);
1409+
14071410
ret = send_cap_msg(&arg);
14081411
if (ret < 0) {
14091412
dout("error sending cap msg, must requeue %p\n", inode);

fs/ceph/snap.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,7 @@ void ceph_queue_cap_snap(struct ceph_inode_info *ci)
464464
struct inode *inode = &ci->vfs_inode;
465465
struct ceph_cap_snap *capsnap;
466466
struct ceph_snap_context *old_snapc, *new_snapc;
467+
struct ceph_buffer *old_blob = NULL;
467468
int used, dirty;
468469

469470
capsnap = kzalloc(sizeof(*capsnap), GFP_NOFS);
@@ -540,7 +541,7 @@ void ceph_queue_cap_snap(struct ceph_inode_info *ci)
540541
capsnap->gid = inode->i_gid;
541542

542543
if (dirty & CEPH_CAP_XATTR_EXCL) {
543-
__ceph_build_xattrs_blob(ci);
544+
old_blob = __ceph_build_xattrs_blob(ci);
544545
capsnap->xattr_blob =
545546
ceph_buffer_get(ci->i_xattrs.blob);
546547
capsnap->xattr_version = ci->i_xattrs.version;
@@ -583,6 +584,7 @@ void ceph_queue_cap_snap(struct ceph_inode_info *ci)
583584
}
584585
spin_unlock(&ci->i_ceph_lock);
585586

587+
ceph_buffer_put(old_blob);
586588
kfree(capsnap);
587589
ceph_put_snap_context(old_snapc);
588590
}

fs/ceph/super.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -924,7 +924,7 @@ extern int ceph_getattr(const struct path *path, struct kstat *stat,
924924
int __ceph_setxattr(struct inode *, const char *, const void *, size_t, int);
925925
ssize_t __ceph_getxattr(struct inode *, const char *, void *, size_t);
926926
extern ssize_t ceph_listxattr(struct dentry *, char *, size_t);
927-
extern void __ceph_build_xattrs_blob(struct ceph_inode_info *ci);
927+
extern struct ceph_buffer *__ceph_build_xattrs_blob(struct ceph_inode_info *ci);
928928
extern void __ceph_destroy_xattrs(struct ceph_inode_info *ci);
929929
extern void __init ceph_xattr_init(void);
930930
extern void ceph_xattr_exit(void);

fs/ceph/xattr.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -752,12 +752,15 @@ static int __get_required_blob_size(struct ceph_inode_info *ci, int name_size,
752752

753753
/*
754754
* If there are dirty xattrs, reencode xattrs into the prealloc_blob
755-
* and swap into place.
755+
* and swap into place. It returns the old i_xattrs.blob (or NULL) so
756+
* that it can be freed by the caller as the i_ceph_lock is likely to be
757+
* held.
756758
*/
757-
void __ceph_build_xattrs_blob(struct ceph_inode_info *ci)
759+
struct ceph_buffer *__ceph_build_xattrs_blob(struct ceph_inode_info *ci)
758760
{
759761
struct rb_node *p;
760762
struct ceph_inode_xattr *xattr = NULL;
763+
struct ceph_buffer *old_blob = NULL;
761764
void *dest;
762765

763766
dout("__build_xattrs_blob %p\n", &ci->vfs_inode);
@@ -788,12 +791,14 @@ void __ceph_build_xattrs_blob(struct ceph_inode_info *ci)
788791
dest - ci->i_xattrs.prealloc_blob->vec.iov_base;
789792

790793
if (ci->i_xattrs.blob)
791-
ceph_buffer_put(ci->i_xattrs.blob);
794+
old_blob = ci->i_xattrs.blob;
792795
ci->i_xattrs.blob = ci->i_xattrs.prealloc_blob;
793796
ci->i_xattrs.prealloc_blob = NULL;
794797
ci->i_xattrs.dirty = false;
795798
ci->i_xattrs.version++;
796799
}
800+
801+
return old_blob;
797802
}
798803

799804
static inline int __get_request_mask(struct inode *in) {

0 commit comments

Comments
 (0)