[Cluster-devel] 回复: [PATCH v1] fs:dlm:Fix NULL pointer dereference bug in accept_from_sock()
Thank you. I see. I hadn't noticed what the add_sock() function did before, but
now I understand that newcon->sock will not be a null pointer.
-邮件原件-
发件人: Alexander Aring
发送时间: 2023年7月5日 0:22
收件人: 王明-软件底层技术部
抄送: Christine Caulfield ; David Teigland
; cluster-devel@redhat.com; linux-ker...@vger.kernel.org;
opensource.kernel
主题: Re: [Cluster-devel] [PATCH v1] fs:dlm:Fix NULL pointer dereference bug in
accept_from_sock()
[你通常不会收到来自 aahri...@redhat.com 的电子邮件。请访问
https://aka.ms/LearnAboutSenderIdentification，以了解这一点为什么很重要]
Hi,
On Tue, Jul 4, 2023 at 6:56 AM Wang Ming wrote:
>
> newcon -> sock is NULL but dereferenced.
> First check newcon. Whether sock is a null pointer.
> If so, the subsequent operations are skipped.
> If it is not empty, perform subsequent operations.
>
did you experience some null pointer dereference? If so, on which kernel was it?
> Signed-off-by: Wang Ming
> ---
> fs/dlm/lowcomms.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index
> 9f14ea9f6..ea18b9478 100644
> --- a/fs/dlm/lowcomms.c
> +++ b/fs/dlm/lowcomms.c
> @@ -1081,9 +1081,12 @@ static int accept_from_sock(void)
> add_sock(newsock, newcon);
>
Here in add_sock() we assign newcon->sock = newsock. It cannot fail and newsock
cannot be null, so holding the newcon->sock_lock write protected _should_ be
safe that others don't manipulate newcon->sock.
It should, that's why I am asking if you experienced some issue here?
> /* check if we receved something while adding */
> - lock_sock(newcon->sock->sk);
> - lowcomms_queue_rwork(newcon);
> - release_sock(newcon->sock->sk);
see above, newcon->sock should always be set at this point.
> + if (newcon->sock) {
> + lock_sock(newcon->sock->sk);
> + lowcomms_queue_rwork(newcon);
> + release_sock(newcon->sock->sk);
> + }
> +
> }
> up_write(>sock_lock);
> srcu_read_unlock(_srcu, idx);
- Alex
Re: [Cluster-devel] [GIT PULL] gfs2 fixes
The pull request you sent on Tue, 4 Jul 2023 14:51:11 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git > tags/gfs2-v6.4-rc5-fixes has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/94c76955e86a5a4f16a1d690b66dcc268c156e6a Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [Cluster-devel] [PATCH v1] fs:dlm:Fix NULL pointer dereference bug in accept_from_sock()
Hi,
On Tue, Jul 4, 2023 at 6:56 AM Wang Ming wrote:
>
> newcon -> sock is NULL but dereferenced.
> First check newcon. Whether sock is a null pointer.
> If so, the subsequent operations are skipped.
> If it is not empty, perform subsequent operations.
>
did you experience some null pointer dereference? If so, on which kernel was it?
> Signed-off-by: Wang Ming
> ---
> fs/dlm/lowcomms.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
> index 9f14ea9f6..ea18b9478 100644
> --- a/fs/dlm/lowcomms.c
> +++ b/fs/dlm/lowcomms.c
> @@ -1081,9 +1081,12 @@ static int accept_from_sock(void)
> add_sock(newsock, newcon);
>
Here in add_sock() we assign newcon->sock = newsock. It cannot fail
and newsock cannot be null, so holding the newcon->sock_lock write
protected _should_ be safe that others don't manipulate newcon->sock.
It should, that's why I am asking if you experienced some issue here?
> /* check if we receved something while adding */
> - lock_sock(newcon->sock->sk);
> - lowcomms_queue_rwork(newcon);
> - release_sock(newcon->sock->sk);
see above, newcon->sock should always be set at this point.
> + if (newcon->sock) {
> + lock_sock(newcon->sock->sk);
> + lowcomms_queue_rwork(newcon);
> + release_sock(newcon->sock->sk);
> + }
> +
> }
> up_write(>sock_lock);
> srcu_read_unlock(_srcu, idx);
- Alex
[Cluster-devel] [GIT PULL] gfs2 fixes
Hi Linus,
please consider pulling the following gfs2 fixes.
Thanks a lot,
Andreas
The following changes since commit 0bdd0f0bf17c5aac16f348ee4b1ebf23d1ec1649:
Merge tag 'gfs2-v6.4-rc4-fix' of
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2 (2023-06-06
05:49:06 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-v6.4-rc5-fixes
for you to fetch changes up to 432928c9377959684c748a9bc6553ed2d3c2ea4f:
gfs2: Add quota_change type (2023-07-03 22:30:48 +0200)
gfs2 fixes
- Move the freeze/thaw logic from glock callback context to process /
worker thread context to prevent deadlocks.
- Fix a quota reference couting bug in do_qc().
- Carry on deallocating inodes even when gfs2_rindex_update() fails.
- Retry filesystem-internal reads when they are interruped by a signal.
- Eliminate kmap_atomic() in favor of kmap_local_page() /
memcpy_{from,to}_page().
- Get rid of noop_direct_IO.
- And a few more minor fixes and cleanups.
Andreas Gruenbacher (12):
gfs2: retry interrupted internal reads
gfs2: Rename remaining "transaction" glock references
gfs2: Rename the {freeze,thaw}_super callbacks
gfs2: Rename gfs2_freeze_lock{ => _shared }
gfs2: Reconfiguring frozen filesystem already rejected
gfs2: Rename SDF_{FS_FROZEN => FREEZE_INITIATOR}
gfs2: Rework freeze / thaw logic
gfs2: Replace sd_freeze_state with SDF_FROZEN flag
gfs2: gfs2_freeze_lock_shared cleanup
gfs: Get rid of unnucessary locking in inode_go_dump
gfs2: Convert remaining kmap_atomic calls to kmap_local_page
gfs2: Use memcpy_{from,to}_page where appropriate
Bob Peterson (8):
gfs2: simplify gdlm_put_lock with out_free label
gfs2: fix minor comment typos
gfs2: ignore rindex_update failure in dinode_dealloc
gfs2: Fix gfs2_qa_get imbalance in gfs2_quota_hold
gfs2: Update rl_unlinked before releasing rgrp lock
gfs2: Don't remember delete unless it's successful
gfs2: Fix duplicate should_fault_in_pages() call
gfs2: Add quota_change type
Christoph Hellwig (1):
gfs2: set FMODE_CAN_ODIRECT instead of a dummy direct_IO method
Deepak R Varma (1):
gfs2: Replace deprecated kmap_atomic with kmap_local_page
Tuo Li (1):
gfs2: Fix possible data races in gfs2_show_options()
fs/gfs2/aops.c | 19 +++--
fs/gfs2/bmap.c | 4 +-
fs/gfs2/file.c | 5 +-
fs/gfs2/glock.c | 4 +-
fs/gfs2/glops.c | 69 ++---
fs/gfs2/incore.h | 12 +--
fs/gfs2/lock_dlm.c | 23 +++---
fs/gfs2/log.c| 11 +--
fs/gfs2/lops.c | 21 +++--
fs/gfs2/ops_fstype.c | 15 +---
fs/gfs2/quota.c | 26 ---
fs/gfs2/recovery.c | 28 +++
fs/gfs2/rgrp.c | 2 +-
fs/gfs2/super.c | 215 ---
fs/gfs2/super.h | 1 +
fs/gfs2/sys.c| 4 +-
fs/gfs2/trans.c | 3 +-
fs/gfs2/util.c | 49
fs/gfs2/util.h | 3 +-
19 files changed, 277 insertions(+), 237 deletions(-)
[Cluster-devel] [PATCH v1] fs:dlm:Fix NULL pointer dereference bug in accept_from_sock()
newcon -> sock is NULL but dereferenced.
First check newcon. Whether sock is a null pointer.
If so, the subsequent operations are skipped.
If it is not empty, perform subsequent operations.
Signed-off-by: Wang Ming
---
fs/dlm/lowcomms.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index 9f14ea9f6..ea18b9478 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -1081,9 +1081,12 @@ static int accept_from_sock(void)
add_sock(newsock, newcon);
/* check if we receved something while adding */
- lock_sock(newcon->sock->sk);
- lowcomms_queue_rwork(newcon);
- release_sock(newcon->sock->sk);
+ if (newcon->sock) {
+ lock_sock(newcon->sock->sk);
+ lowcomms_queue_rwork(newcon);
+ release_sock(newcon->sock->sk);
+ }
+
}
up_write(>sock_lock);
srcu_read_unlock(_srcu, idx);
--
2.25.1
