From a034c1370ded2ae6cbdc73a78241b3ed98c86d3d Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Mon, 6 Mar 2023 15:48:08 -0500 Subject: [PATCH] fs: dlm: fix DLM_IFL_CB_PENDING gets overwritten This patch introduce a new internal flag per lkb value to handle internal flags which are handled not on wire. The current lkb internal flags stored as lkb->lkb_flags are split in upper and lower bits, the lower bits are used to share internal flags over wire for other cluster wide lkb copies on other nodes. In commit 61bed0baa4db ("fs: dlm: use a non-static queue for callbacks") we introduced a new internal flag for pending callbacks for the dlm callback queue. This flag is protected by the lkb->lkb_cb_lock lock. This patch overlooked that on dlm receive path and the mentioned upper and lower bits, that dlm will read the flags, mask it and write it back. As example receive_flags() in fs/dlm/lock.c. This flag manipulation is not done atomically and is not protected by lkb->lkb_cb_lock. This has unknown side effects of the current callback handling. In future we should move to set/clear/test bit functionality and avoid read, mask and writing back flag values. In later patches we will move the upper parts to the new introduced internal lkb flags which are not shared between other cluster nodes to the new non shared internal flag field to avoid similar issues. Cc: stable@vger.kernel.org Fixes: 61bed0baa4db ("fs: dlm: use a non-static queue for callbacks") Reported-by: Bob Peterson Signed-off-by: Alexander Aring Signed-off-by: David Teigland --- fs/dlm/ast.c | 9 ++++----- fs/dlm/dlm_internal.h | 5 ++++- fs/dlm/user.c | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/fs/dlm/ast.c b/fs/dlm/ast.c index 26fef99..39805ae 100644 --- a/fs/dlm/ast.c +++ b/fs/dlm/ast.c @@ -45,7 +45,7 @@ void dlm_purge_lkb_callbacks(struct dlm_lkb *lkb) kref_put(&cb->ref, dlm_release_callback); } - lkb->lkb_flags &= ~DLM_IFL_CB_PENDING; + clear_bit(DLM_IFL_CB_PENDING_BIT, &lkb->lkb_iflags); /* invalidate */ dlm_callback_set_last_ptr(&lkb->lkb_last_cast, NULL); @@ -103,10 +103,9 @@ int dlm_enqueue_lkb_callback(struct dlm_lkb *lkb, uint32_t flags, int mode, cb->sb_status = status; cb->sb_flags = (sbflags & 0x000000FF); kref_init(&cb->ref); - if (!(lkb->lkb_flags & DLM_IFL_CB_PENDING)) { - lkb->lkb_flags |= DLM_IFL_CB_PENDING; + if (!test_and_set_bit(DLM_IFL_CB_PENDING_BIT, &lkb->lkb_iflags)) rv = DLM_ENQUEUE_CALLBACK_NEED_SCHED; - } + list_add_tail(&cb->list, &lkb->lkb_callbacks); if (flags & DLM_CB_CAST) @@ -209,7 +208,7 @@ void dlm_callback_work(struct work_struct *work) spin_lock(&lkb->lkb_cb_lock); rv = dlm_dequeue_lkb_callback(lkb, &cb); if (rv == DLM_DEQUEUE_CALLBACK_EMPTY) { - lkb->lkb_flags &= ~DLM_IFL_CB_PENDING; + clear_bit(DLM_IFL_CB_PENDING_BIT, &lkb->lkb_iflags); spin_unlock(&lkb->lkb_cb_lock); break; } diff --git a/fs/dlm/dlm_internal.h b/fs/dlm/dlm_internal.h index ab1a553..9bf7096 100644 --- a/fs/dlm/dlm_internal.h +++ b/fs/dlm/dlm_internal.h @@ -211,7 +211,9 @@ struct dlm_args { #endif #define DLM_IFL_DEADLOCK_CANCEL 0x01000000 #define DLM_IFL_STUB_MS 0x02000000 /* magic number for m_flags */ -#define DLM_IFL_CB_PENDING 0x04000000 + +#define DLM_IFL_CB_PENDING_BIT 0 + /* least significant 2 bytes are message changed, they are full transmitted * but at receive side only the 2 bytes LSB will be set. * @@ -246,6 +248,7 @@ struct dlm_lkb { uint32_t lkb_exflags; /* external flags from caller */ uint32_t lkb_sbflags; /* lksb flags */ uint32_t lkb_flags; /* internal flags */ + unsigned long lkb_iflags; /* internal flags */ uint32_t lkb_lvbseq; /* lvb sequence number */ int8_t lkb_status; /* granted, waiting, convert */ diff --git a/fs/dlm/user.c b/fs/dlm/user.c index 3512950..688a480 100644 --- a/fs/dlm/user.c +++ b/fs/dlm/user.c @@ -884,7 +884,7 @@ static ssize_t device_read(struct file *file, char __user *buf, size_t count, goto try_another; case DLM_DEQUEUE_CALLBACK_LAST: list_del_init(&lkb->lkb_cb_list); - lkb->lkb_flags &= ~DLM_IFL_CB_PENDING; + clear_bit(DLM_IFL_CB_PENDING_BIT, &lkb->lkb_iflags); break; case DLM_DEQUEUE_CALLBACK_SUCCESS: break; -- 2.7.4