github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-09-22 12:44:03 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

ocfs2: fix circular locking dependency in ocfs2_init_acl()

Browse Source 
A lockdep warning indicates a circular locking dependency between
`&oi->ip_xattr_sem` and `&journal->j_trans_barrier`:

WARNING: possible circular locking dependency detected
is trying to acquire lock:
 (&oi->ip_xattr_sem){++++}-{4:4}, at: ocfs2_init_acl+0x2fd/0x7e0
 fs/ocfs2/acl.c:367

but task is already holding lock:
 (&journal->j_trans_barrier){.+.+}-{4:4}, at: ocfs2_start_trans+0x3ab/0x700
 fs/ocfs2/journal.c:369

The deadlock involves two code paths: Path 1 (setxattr) where
`ocfs2_xattr_set()` acquires `ip_xattr_sem` (write) and then starts a
transaction, which acquires `j_trans_barrier` (read); and Path 2
(mkdir/mknod) where `ocfs2_mknod()` starts a transaction (`j_trans_barrier`
read) and then calls `ocfs2_init_acl()`, which attempts to acquire
`ip_xattr_sem` (read) on the parent directory to retrieve the default ACL.

Because rw_semaphores are subject to writer priority, a pending writer on
`j_trans_barrier` (e.g., the journal commit thread) can cause Path 1 to
block, while Path 2 is blocked waiting for Path 1 to release
`ip_xattr_sem`.

The patch fixes the lock ordering by precomputing the ACL state before
starting the OCFS2 transaction, while preserving POSIX ACL storage
semantics and the existing inode/security initialization order. By reading
the parent directory's default ACL and preparing the new inode's ACLs
outside the transaction, `ip_xattr_sem` is always acquired before
`j_trans_barrier`.

`struct ocfs2_acl_state` encapsulates the prepared ACL state, while
`ocfs2_acl_init_prepare()` and `ocfs2_acl_init_release()` avoid code
duplication between `ocfs2_mknod()` and `ocfs2_init_security_and_acl()`.
`ocfs2_calc_xattr_init()` and `ocfs2_init_acl()` use this precomputed
state, removing internal `ip_xattr_sem` acquisition and redundant disk
reads.

Additionally, remove the `ip_xattr_sem` acquisition from
`ocfs2_xattr_set_handle()`. This function is only used while initializing a
new inode that has not yet been inserted into the inode hash or attached to
a dentry, meaning there is no risk of concurrent access and the lock is
unnecessary.

Link: https://lore.kernel.org/4094de06-9b69-4174-b2ee-08126dffc693@mail.kernel.org
Fixes: 16c8d569f5 ("ocfs2/acl: use 'ip_xattr_sem' to protect getting extended attribute")
Signed-off-by: Krystian Kaniewski <krystianmkaniewski@gmail.com>
Assisted-by: Gemini:gemini-3.5-flash Gemini:gemini-3.1-pro-preview syzbot
Reported-by: syzbot+4007ab5229e732466d9f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4007ab5229e732466d9f
Link: https://syzkaller.appspot.com/ai_job?id=cc75363d-c672-499e-8fc5-44bcdc1cee39
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Cc: Mark Fasheh <mark@fasheh.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Junxiao Bi <junxiao.bi@oracle.com>
Cc: Changwei Ge <gechangwei@live.cn>
Cc: Jun Piao <piaojun@huawei.com>
Cc: Heming Zhao <heming.zhao@suse.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
This commit is contained in:
Krystian Kaniewski 2026-07-30 07:42:21 +00:00 committed by Andrew Morton
parent bb88131c98
commit bd7c05fb4a
5 changed files with 176 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

141
fs/ocfs2/acl.c
View File

@ -110,8 +110,7 @@ static void *ocfs2_acl_to_xattr(const struct posix_acl *acl, size_t *size)
return ocfs2_acl;
}
static struct posix_acl *ocfs2_get_acl_nolock(struct inode *inode,
int type,
static struct posix_acl *ocfs2_get_acl_nolock(struct inode *inode, int type,
struct buffer_head *di_bh)
{
int name_index;
@ -349,63 +348,105 @@ int ocfs2_acl_chmod(struct inode *inode, struct buffer_head *bh)
* Initialize the ACLs of a new inode. If parent directory has default ACL,
* then clone to new inode. Called from ocfs2_mknod.
*/
int ocfs2_init_acl(handle_t *handle,
struct inode *inode,
struct inode *dir,
struct buffer_head *di_bh,
struct buffer_head *dir_bh,
struct ocfs2_alloc_context *meta_ac,
struct ocfs2_alloc_context *data_ac)
void ocfs2_acl_init_release(struct ocfs2_acl_state *state)
{
posix_acl_release(state->default_acl);
posix_acl_release(state->acl);
state->default_acl = NULL;
state->acl = NULL;
}
int ocfs2_acl_init_prepare(struct inode *inode, struct inode *dir,
struct buffer_head *dir_bh,
struct ocfs2_acl_state *state)
{
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
struct posix_acl *acl = NULL;
int ret = 0, ret2;
umode_t mode;
int ret = 0;
if (!S_ISLNK(inode->i_mode)) {
if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
down_read(&OCFS2_I(dir)->ip_xattr_sem);
acl = ocfs2_get_acl_nolock(dir, ACL_TYPE_DEFAULT,
dir_bh);
up_read(&OCFS2_I(dir)->ip_xattr_sem);
if (IS_ERR(acl))
return PTR_ERR(acl);
state->default_acl = NULL;
state->acl = NULL;
state->mode = inode->i_mode;
if (S_ISLNK(inode->i_mode))
return 0;
if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
down_read(&OCFS2_I(dir)->ip_xattr_sem);
state->default_acl =
ocfs2_get_acl_nolock(dir, ACL_TYPE_DEFAULT, dir_bh);
up_read(&OCFS2_I(dir)->ip_xattr_sem);
if (IS_ERR(state->default_acl)) {
ret = PTR_ERR(state->default_acl);
state->default_acl = NULL;
return ret;
}
if (!acl) {
mode = inode->i_mode & ~current_umask();
ret = ocfs2_acl_set_mode(inode, di_bh, handle, mode);
if (ret) {
mlog_errno(ret);
if (state->default_acl) {
state->acl = posix_acl_dup(state->default_acl);
if (!state->acl) {
ret = -ENOMEM;
goto cleanup;
}
}
}
if ((osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) && acl) {
if (S_ISDIR(inode->i_mode)) {
ret = ocfs2_set_acl(handle, inode, di_bh,
ACL_TYPE_DEFAULT, acl,
meta_ac, data_ac);
if (ret)
ret = __posix_acl_create(&state->acl, GFP_NOFS,
&state->mode);
if (ret < 0)
goto cleanup;
if (ret == 0) {
posix_acl_release(state->acl);
state->acl = NULL;
}
if (!S_ISDIR(inode->i_mode)) {
posix_acl_release(state->default_acl);
state->default_acl = NULL;
}
} else {
state->mode &= ~current_umask();
}
mode = inode->i_mode;
ret = __posix_acl_create(&acl, GFP_NOFS, &mode);
if (ret < 0)
return ret;
ret2 = ocfs2_acl_set_mode(inode, di_bh, handle, mode);
if (ret2) {
mlog_errno(ret2);
ret = ret2;
goto cleanup;
}
if (ret > 0) {
ret = ocfs2_set_acl(handle, inode,
di_bh, ACL_TYPE_ACCESS,
acl, meta_ac, data_ac);
}
} else {
state->mode &= ~current_umask();
}
return 0;
cleanup:
posix_acl_release(acl);
ocfs2_acl_init_release(state);
return ret;
}
int ocfs2_init_acl(handle_t *handle, struct inode *inode,
struct buffer_head *di_bh,
struct ocfs2_alloc_context *meta_ac,
struct ocfs2_alloc_context *data_ac,
struct ocfs2_acl_state *state)
{
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
int ret = 0;
if (S_ISLNK(inode->i_mode))
return 0;
if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
if (S_ISDIR(inode->i_mode) && state->default_acl) {
ret = ocfs2_set_acl(handle, inode, di_bh,
ACL_TYPE_DEFAULT,
state->default_acl, meta_ac,
data_ac);
if (ret)
return ret;
}
}
ret = ocfs2_acl_set_mode(inode, di_bh, handle, state->mode);
if (ret) {
mlog_errno(ret);
return ret;
}
if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
if (state->acl) {
ret = ocfs2_set_acl(handle, inode, di_bh,
ACL_TYPE_ACCESS, state->acl,
meta_ac, data_ac);
}
}
return ret;
}

19
fs/ocfs2/acl.h
View File

@ -20,9 +20,20 @@ struct posix_acl *ocfs2_iop_get_acl(struct inode *inode, int type, bool rcu);
int ocfs2_iop_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
struct posix_acl *acl, int type);
extern int ocfs2_acl_chmod(struct inode *, struct buffer_head *);
extern int ocfs2_init_acl(handle_t *, struct inode *, struct inode *,
struct buffer_head *, struct buffer_head *,
struct ocfs2_alloc_context *,
struct ocfs2_alloc_context *);
struct ocfs2_acl_state {
struct posix_acl *default_acl;
struct posix_acl *acl;
umode_t mode;
};
int ocfs2_acl_init_prepare(struct inode *inode, struct inode *dir,
struct buffer_head *dir_bh,
struct ocfs2_acl_state *state);
void ocfs2_acl_init_release(struct ocfs2_acl_state *state);
int ocfs2_init_acl(handle_t *handle, struct inode *inode,
struct buffer_head *di_bh,
struct ocfs2_alloc_context *meta_ac,
struct ocfs2_alloc_context *data_ac,
struct ocfs2_acl_state *state);
#endif /* OCFS2_ACL_H */

17
fs/ocfs2/namei.c
View File

@ -256,6 +256,7 @@ static int ocfs2_mknod(struct mnt_idmap *idmap,
sigset_t oldset;
int did_block_signals = 0;
struct ocfs2_dentry_lock *dl = NULL;
struct ocfs2_acl_state acl_state = { 0 };
trace_ocfs2_mknod(dir, dentry, dentry->d_name.len, dentry->d_name.name,
(unsigned long long)OCFS2_I(dir)->ip_blkno,
@ -330,10 +331,14 @@ static int ocfs2_mknod(struct mnt_idmap *idmap,
}
}
status = ocfs2_acl_init_prepare(inode, dir, parent_fe_bh, &acl_state);
if (status < 0)
goto leave;
/* calculate meta data/clusters for setting security and acl xattr */
status = ocfs2_calc_xattr_init(dir, parent_fe_bh, mode,
&si, &want_clusters,
&xattr_credits, &want_meta);
status = ocfs2_calc_xattr_init(dir, mode, &si, &want_clusters,
&xattr_credits, &want_meta,
&acl_state);
if (status < 0) {
mlog_errno(status);
goto leave;
@ -411,8 +416,8 @@ static int ocfs2_mknod(struct mnt_idmap *idmap,
inc_nlink(dir);
}
status = ocfs2_init_acl(handle, inode, dir, new_fe_bh, parent_fe_bh,
meta_ac, data_ac);
status = ocfs2_init_acl(handle, inode, new_fe_bh, meta_ac, data_ac,
&acl_state);
if (status < 0) {
mlog_errno(status);
@ -477,6 +482,8 @@ static int ocfs2_mknod(struct mnt_idmap *idmap,
brelse(parent_fe_bh);
kfree(si.value);
ocfs2_acl_init_release(&acl_state);
ocfs2_free_dir_lookup_result(&lookup);
if (inode_ac)

86
fs/ocfs2/xattr.c
View File

@ -635,13 +635,10 @@ int ocfs2_calc_security_init(struct inode *dir,
return ret;
}
int ocfs2_calc_xattr_init(struct inode *dir,
struct buffer_head *dir_bh,
umode_t mode,
int ocfs2_calc_xattr_init(struct inode *dir, umode_t mode,
struct ocfs2_security_xattr_info *si,
int *want_clusters,
int *xattr_credits,
int *want_meta)
int *want_clusters, int *xattr_credits,
int *want_meta, struct ocfs2_acl_state *acl_state)
{
int ret = 0;
struct ocfs2_super *osb = OCFS2_SB(dir->i_sb);
@ -652,19 +649,15 @@ int ocfs2_calc_xattr_init(struct inode *dir,
si->value_len);
if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
down_read(&OCFS2_I(dir)->ip_xattr_sem);
acl_len = ocfs2_xattr_get_nolock(dir, dir_bh,
OCFS2_XATTR_INDEX_POSIX_ACL_DEFAULT,
"", NULL, 0);
up_read(&OCFS2_I(dir)->ip_xattr_sem);
if (acl_len > 0) {
a_size = ocfs2_xattr_entry_real_size(0, acl_len);
if (S_ISDIR(mode))
a_size <<= 1;
} else if (acl_len != 0 && acl_len != -ENODATA) {
ret = acl_len;
mlog_errno(ret);
return ret;
if (acl_state->default_acl && S_ISDIR(mode)) {
acl_len = acl_state->default_acl->a_count *
sizeof(struct ocfs2_acl_entry);
a_size += ocfs2_xattr_entry_real_size(0, acl_len);
}
if (acl_state->acl) {
acl_len = acl_state->acl->a_count *
sizeof(struct ocfs2_acl_entry);
a_size += ocfs2_xattr_entry_real_size(0, acl_len);
}
}
@ -707,14 +700,33 @@ int ocfs2_calc_xattr_init(struct inode *dir,
new_clusters);
*want_clusters += new_clusters;
}
if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL &&
acl_len > OCFS2_XATTR_INLINE_SIZE) {
/* for directory, it has DEFAULT and ACCESS two types of acls */
new_clusters = (S_ISDIR(mode) ? 2 : 1) *
ocfs2_clusters_for_bytes(dir->i_sb, acl_len);
*xattr_credits += ocfs2_clusters_to_blocks(dir->i_sb,
new_clusters);
*want_clusters += new_clusters;
if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
if (acl_state->default_acl && S_ISDIR(mode)) {
acl_len = acl_state->default_acl->a_count *
sizeof(struct ocfs2_acl_entry);
if (acl_len > OCFS2_XATTR_INLINE_SIZE) {
new_clusters =
ocfs2_clusters_for_bytes(dir->i_sb,
acl_len);
*xattr_credits +=
ocfs2_clusters_to_blocks(dir->i_sb,
new_clusters);
*want_clusters += new_clusters;
}
}
if (acl_state->acl) {
acl_len = acl_state->acl->a_count *
sizeof(struct ocfs2_acl_entry);
if (acl_len > OCFS2_XATTR_INLINE_SIZE) {
new_clusters =
ocfs2_clusters_for_bytes(dir->i_sb,
acl_len);
*xattr_credits +=
ocfs2_clusters_to_blocks(dir->i_sb,
new_clusters);
*want_clusters += new_clusters;
}
}
}
return ret;
@ -3678,9 +3690,10 @@ static int __ocfs2_xattr_set_handle(struct inode *inode,
}
/*
* This function only called duing creating inode
* for init security/acl xattrs of the new inode.
* All transanction credits have been reserved in mknod.
* This helper is only for setting initial ACL or security xattrs on an inode
* that is still unpublished, unhashed, and unattached to a dentry.
* Ordinary xattr updates must use ocfs2_xattr_set().
* All transaction credits have been reserved in mknod or symlink callers.
*/
int ocfs2_xattr_set_handle(handle_t *handle,
struct inode *inode,
@ -3737,8 +3750,6 @@ int ocfs2_xattr_set_handle(handle_t *handle,
xis.inode_bh = xbs.inode_bh = di_bh;
di = (struct ocfs2_dinode *)di_bh->b_data;
down_write(&OCFS2_I(inode)->ip_xattr_sem);
ret = ocfs2_xattr_ibody_find(inode, name_index, name, &xis);
if (ret)
goto cleanup;
@ -3751,7 +3762,6 @@ int ocfs2_xattr_set_handle(handle_t *handle,
ret = __ocfs2_xattr_set_handle(inode, di, &xi, &xis, &xbs, &ctxt);
cleanup:
up_write(&OCFS2_I(inode)->ip_xattr_sem);
brelse(xbs.xattr_bh);
ocfs2_xattr_bucket_free(xbs.bucket);
@ -7451,6 +7461,7 @@ int ocfs2_init_security_and_acl(struct inode *dir,
{
int ret = 0;
struct buffer_head *dir_bh = NULL;
struct ocfs2_acl_state acl_state = { 0 };
ret = ocfs2_init_security_get(inode, dir, qstr, NULL);
if (ret) {
@ -7463,10 +7474,17 @@ int ocfs2_init_security_and_acl(struct inode *dir,
mlog_errno(ret);
goto leave;
}
ret = ocfs2_init_acl(NULL, inode, dir, NULL, dir_bh, NULL, NULL);
ret = ocfs2_acl_init_prepare(inode, dir, dir_bh, &acl_state);
if (ret)
goto unlock;
ret = ocfs2_init_acl(NULL, inode, NULL, NULL, NULL, &acl_state);
if (ret)
mlog_errno(ret);
unlock:
ocfs2_acl_init_release(&acl_state);
ocfs2_inode_unlock(dir, 0);
brelse(dir_bh);
leave:

9
fs/ocfs2/xattr.h
View File

@ -57,9 +57,12 @@ int ocfs2_init_security_set(handle_t *, struct inode *,
int ocfs2_calc_security_init(struct inode *,
struct ocfs2_security_xattr_info *,
int *, int *, struct ocfs2_alloc_context **);
int ocfs2_calc_xattr_init(struct inode *, struct buffer_head *,
umode_t, struct ocfs2_security_xattr_info *,
int *, int *, int *);
struct ocfs2_acl_state;
int ocfs2_calc_xattr_init(struct inode *dir, umode_t mode,
struct ocfs2_security_xattr_info *si,
int *want_clusters, int *xattr_credits,
int *want_meta, struct ocfs2_acl_state *acl_state);
/*
* xattrs can live inside an inode, as part of an external xattr block,