Skip to contentSkip to conversation

dlm: Treat dlm_local_addr[0] as sockaddr_storage *

PATCH In review Jordan Rife 2 participants 7 messages Feb 9, 2024 – Feb 13, 2024 Stable branch

lore
Share & tools
lore

Jordan Rife

author
#0 lore raw

PATCH 1/1 dlm: Treat dlm_local_addr[0] as sockaddr_storage *

Backport e11dea8 ("dlm: use kernel_connect() and kernel_bind()") to Linux stable 6.1 caused a regression. The original patch expected dlm_local_addrs[0] to be of type sockaddr_storage, because c51c9cd ("fs:
dlm: don't put dlm_local_addrs on heap") changed its type from sockaddr_storage* to sockaddr_storage in Linux 6.5+ while in older Linux versions this is still the original sockaddr_storage*.

Link https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1063338
Cc <[email protected]> # 6.1.x
Fixes e11dea8f5033 ("dlm: use kernel_connect() and kernel_bind()")
Signed-off-by Jordan Rife <[email protected]>
---
 fs/dlm/lowcomms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/dlm/lowcomms.c +1 -1
index 72f34f96d0155..8426073e73cf2 100644
@@ -1900,7 +1900,7 @@ static int dlm_tcp_listen_bind(struct socket *sock)
 
 	/* Bind to our port */
 	make_sockaddr(dlm_local_addr[0], dlm_config.ci_tcp_port, &addr_len);
-	return kernel_bind(sock, (struct sockaddr *)&dlm_local_addr[0],
+	return kernel_bind(sock, (struct sockaddr *)dlm_local_addr[0],
 			   addr_len);
 }
Signature · 1 line
2.43.0.687.g38aa6559b0-goog
maintainer
#1 lore raw
Quoted 6 lines · Jordan Rife

Backport e11dea8 ("dlm: use kernel_connect() and kernel_bind()") to Linux stable 6.1 caused a regression. The original patch expected dlm_local_addrs[0] to be of type sockaddr_storage, because c51c9cd ("fs:
dlm: don't put dlm_local_addrs on heap") changed its type from sockaddr_storage* to sockaddr_storage in Linux 6.5+ while in older Linux versions this is still the original sockaddr_storage*.

Or we can just take c51c9cd8addc ("fs: dlm: don't put dlm_local_addrs on heap") into the relevant trees?

Signature · 2 lines
Thanks,
Sasha
author
#2 lore raw
Quoted 9 lines · Sasha Levin
Quoted 6 lines · Jordan Rife

Backport e11dea8 ("dlm: use kernel_connect() and kernel_bind()") to Linux stable 6.1 caused a regression. The original patch expected dlm_local_addrs[0] to be of type sockaddr_storage, because c51c9cd ("fs:
dlm: don't put dlm_local_addrs on heap") changed its type from sockaddr_storage* to sockaddr_storage in Linux 6.5+ while in older Linux versions this is still the original sockaddr_storage*.

Quoted 6 lines

Or we can just take c51c9cd8addc ("fs: dlm: don't put dlm_local_addrs on heap") into the relevant trees?

--
Thanks,
Sasha

Hi Sasha,

Just my 2c, but backporting c51c9cd8addc ("fs: dlm: don't put dlm_local_addrs on

Quoted 1 line

heap") feels a bit riskier than just correcting the call to kernel_bind(), as it's a much

bigger change. Maybe someone more familiar with the dlm codebase can chime in and say whether or not they are confident with backporting this change.

-Jordan

maintainer
#3 lore raw
Quoted 17 lines · Jordan Rife
Quoted 9 lines · Sasha Levin
Quoted 6 lines · Jordan Rife

Backport e11dea8 ("dlm: use kernel_connect() and kernel_bind()") to Linux stable 6.1 caused a regression. The original patch expected dlm_local_addrs[0] to be of type sockaddr_storage, because c51c9cd ("fs:
dlm: don't put dlm_local_addrs on heap") changed its type from sockaddr_storage* to sockaddr_storage in Linux 6.5+ while in older Linux versions this is still the original sockaddr_storage*.

Quoted 6 lines

Or we can just take c51c9cd8addc ("fs: dlm: don't put dlm_local_addrs on heap") into the relevant trees?

--
Thanks,
Sasha

Quoted 6 lines

Hi Sasha,

Just my 2c, but backporting c51c9cd8addc ("fs: dlm: don't put dlm_local_addrs on

Quoted 1 line

heap") feels a bit riskier than just correcting the call to kernel_bind(), as it's a much

bigger change. Maybe someone more familiar with the dlm codebase can chime in and say whether or not they are confident with backporting this change.

It's a bigger change, but in our experience it's the small fixups that end up carrying the bigger risk.

Backporting the original change also has the advantage of preventing similar issues from happening in the future.

Signature · 2 lines
Thanks,
Sasha
author
#4 lore raw

Sasha,

OK, fair enough. I will send out another patch to backport c51c9cd ("fs:

Quoted 1 line
Quoted 1 line
Quoted 1 line

dlm: don't put dlm_local_addrs on heap") to 6.1.

-Jordan

maintainer
#5 lore raw
Quoted 4 lines · Jordan Rife

Sasha,

OK, fair enough. I will send out another patch to backport c51c9cd ("fs:

Quoted 1 line
Quoted 1 line
Quoted 1 line

dlm: don't put dlm_local_addrs on heap") to 6.1.

Already queued up, thanks :)

Signature · 2 lines
Thanks,
Sasha
author
#6 lore raw

Ack. Thanks.

-Jordan

Quoted 12 lines · Sasha Levin
Quoted 4 lines · Jordan Rife

Sasha,

OK, fair enough. I will send out another patch to backport c51c9cd ("fs:

Quoted 1 line
Quoted 1 line
Quoted 1 line

dlm: don't put dlm_local_addrs on heap") to 6.1.

Already queued up, thanks :)

--
Thanks,
Sasha