[PATCH 6.1.y] dlm: Treat dlm_local_addr[0] as sockaddr_storage *

[PATCH 6.1.y] dlm: Treat dlm_local_addr[0] as sockaddr_storage *

[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*.

Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1063338
Cc: <stable@vger.kernel.org> # 6.1.x
Fixes: e11dea8f5033 ("dlm: use kernel_connect() and kernel_bind()")
Signed-off-by: Jordan Rife <jrife@google.com>
---
 fs/dlm/lowcomms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index 72f34f96d0155..8426073e73cf2 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -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);
 }
 
-- 
2.43.0.687.g38aa6559b0-goog

Re: [PATCH 6.1.y] dlm: Treat dlm_local_addr[0] as sockaddr_storage *

[Sasha Levin]

On Fri, Feb 09, 2024 at 10:26:57AM -0600, Jordan Rife wrote:
>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?

-- 
Thanks,
Sasha

Re: [PATCH 6.1.y] dlm: Treat dlm_local_addr[0] as sockaddr_storage *

[Jordan Rife]

On Fri, Feb 9, 2024 at 10:52 AM Sasha Levin <sashal@kernel.org> wrote:
>
> On Fri, Feb 09, 2024 at 10:26:57AM -0600, Jordan Rife wrote:
> >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?
>
> --
> Thanks,
> Sasha

Hi Sasha,

Just my 2c, but backporting c51c9cd8addc ("fs: dlm: don't put dlm_local_addrs on
> 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

Re: [PATCH 6.1.y] dlm: Treat dlm_local_addr[0] as sockaddr_storage *

[Sasha Levin]

On Fri, Feb 09, 2024 at 11:08:45AM -0800, Jordan Rife wrote:
>On Fri, Feb 9, 2024 at 10:52 AM Sasha Levin <sashal@kernel.org> wrote:
>>
>> On Fri, Feb 09, 2024 at 10:26:57AM -0600, Jordan Rife wrote:
>> >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?
>>
>> --
>> Thanks,
>> Sasha
>
>Hi Sasha,
>
>Just my 2c, but backporting c51c9cd8addc ("fs: dlm: don't put dlm_local_addrs on
>> 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.

-- 
Thanks,
Sasha

Re: [PATCH 6.1.y] dlm: Treat dlm_local_addr[0] as sockaddr_storage *

[Jordan Rife]

Sasha,

OK, fair enough. I will send out another patch to backport c51c9cd ("fs:
>> >dlm: don't put dlm_local_addrs on heap") to 6.1.

-Jordan

Re: [PATCH 6.1.y] dlm: Treat dlm_local_addr[0] as sockaddr_storage *

[Sasha Levin]

On Sun, Feb 11, 2024 at 09:30:02AM -0800, Jordan Rife wrote:
>Sasha,
>
>OK, fair enough. I will send out another patch to backport c51c9cd ("fs:
>>> >dlm: don't put dlm_local_addrs on heap") to 6.1.

Already queued up, thanks :)

-- 
Thanks,
Sasha

Re: [PATCH 6.1.y] dlm: Treat dlm_local_addr[0] as sockaddr_storage *

[Jordan Rife]

Ack. Thanks.

-Jordan

On Tue, Feb 13, 2024 at 4:20 AM Sasha Levin <sashal@kernel.org> wrote:
>
> On Sun, Feb 11, 2024 at 09:30:02AM -0800, Jordan Rife wrote:
> >Sasha,
> >
> >OK, fair enough. I will send out another patch to backport c51c9cd ("fs:
> >>> >dlm: don't put dlm_local_addrs on heap") to 6.1.
>
> Already queued up, thanks :)
>
> --
> Thanks,
> Sasha