* [PATCH net-next] net: Use _K_SS_MAXSIZE instead of absolute value @ 2023-07-19 8:44 Breno Leitao 2023-07-19 17:04 ` Kuniyuki Iwashima 0 siblings, 1 reply; 5+ messages in thread From: Breno Leitao @ 2023-07-19 8:44 UTC (permalink / raw) To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni Cc: leit, Kuniyuki Iwashima, Alexei Starovoitov, Martin KaFai Lau, Alexander Mikhalitsyn, David Howells, Jason Xing, Xin Long, open list:NETWORKING [GENERAL], open list Looking at sk_getsockopt function, it is unclear why 128 is a magical number. Use the proper macro, so it becomes clear to understand what the value mean, and get a reference where it is coming from (user-exported API). Signed-off-by: Breno Leitao <leitao@debian.org> --- net/core/sock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/core/sock.c b/net/core/sock.c index 9370fd50aa2c..58b6f00197d6 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -1815,7 +1815,7 @@ int sk_getsockopt(struct sock *sk, int level, int optname, case SO_PEERNAME: { - char address[128]; + char address[_K_SS_MAXSIZE]; lv = sock->ops->getname(sock, (struct sockaddr *)address, 2); if (lv < 0) -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: Use _K_SS_MAXSIZE instead of absolute value 2023-07-19 8:44 [PATCH net-next] net: Use _K_SS_MAXSIZE instead of absolute value Breno Leitao @ 2023-07-19 17:04 ` Kuniyuki Iwashima 2023-07-19 17:18 ` Breno Leitao 0 siblings, 1 reply; 5+ messages in thread From: Kuniyuki Iwashima @ 2023-07-19 17:04 UTC (permalink / raw) To: leitao Cc: alexander, ast, davem, dhowells, edumazet, kernelxing, kuba, kuniyu, leit, linux-kernel, lucien.xin, martin.lau, netdev, pabeni From: Breno Leitao <leitao@debian.org> Date: Wed, 19 Jul 2023 01:44:12 -0700 > Looking at sk_getsockopt function, it is unclear why 128 is a magical > number. > > Use the proper macro, so it becomes clear to understand what the value > mean, and get a reference where it is coming from (user-exported API). > > Signed-off-by: Breno Leitao <leitao@debian.org> > --- > net/core/sock.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/sock.c b/net/core/sock.c > index 9370fd50aa2c..58b6f00197d6 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -1815,7 +1815,7 @@ int sk_getsockopt(struct sock *sk, int level, int optname, > > case SO_PEERNAME: > { > - char address[128]; > + char address[_K_SS_MAXSIZE]; I guess you saw a bug caught by the fortified memcpy(), but this doesn't fix it properly. I'll post a series soon that fix the issue and another realted one. Thanks! > > lv = sock->ops->getname(sock, (struct sockaddr *)address, 2); > if (lv < 0) > -- > 2.34.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: Use _K_SS_MAXSIZE instead of absolute value 2023-07-19 17:04 ` Kuniyuki Iwashima @ 2023-07-19 17:18 ` Breno Leitao 2023-07-19 17:30 ` Kuniyuki Iwashima 0 siblings, 1 reply; 5+ messages in thread From: Breno Leitao @ 2023-07-19 17:18 UTC (permalink / raw) To: Kuniyuki Iwashima Cc: alexander, ast, davem, dhowells, edumazet, kernelxing, kuba, leit, linux-kernel, lucien.xin, martin.lau, netdev, pabeni On Wed, Jul 19, 2023 at 10:04:45AM -0700, Kuniyuki Iwashima wrote: > From: Breno Leitao <leitao@debian.org> > Date: Wed, 19 Jul 2023 01:44:12 -0700 > > Looking at sk_getsockopt function, it is unclear why 128 is a magical > > number. > > > > Use the proper macro, so it becomes clear to understand what the value > > mean, and get a reference where it is coming from (user-exported API). > > > > Signed-off-by: Breno Leitao <leitao@debian.org> > > --- > > net/core/sock.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/core/sock.c b/net/core/sock.c > > index 9370fd50aa2c..58b6f00197d6 100644 > > --- a/net/core/sock.c > > +++ b/net/core/sock.c > > @@ -1815,7 +1815,7 @@ int sk_getsockopt(struct sock *sk, int level, int optname, > > > > case SO_PEERNAME: > > { > > - char address[128]; > > + char address[_K_SS_MAXSIZE]; > > I guess you saw a bug caught by the fortified memcpy(), but this > doesn't fix it properly. Not really, in fact. I was reading this code, and I found this discussion a while ago, where I got the idea: https://lore.kernel.org/lkml/20140930.005925.995989898229686123.davem@davemloft.net/ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: Use _K_SS_MAXSIZE instead of absolute value 2023-07-19 17:18 ` Breno Leitao @ 2023-07-19 17:30 ` Kuniyuki Iwashima 2023-07-20 9:29 ` Breno Leitao 0 siblings, 1 reply; 5+ messages in thread From: Kuniyuki Iwashima @ 2023-07-19 17:30 UTC (permalink / raw) To: leitao Cc: alexander, ast, davem, dhowells, edumazet, kernelxing, kuba, kuniyu, leit, linux-kernel, lucien.xin, martin.lau, netdev, pabeni From: Breno Leitao <leitao@debian.org> Date: Wed, 19 Jul 2023 10:18:49 -0700 > On Wed, Jul 19, 2023 at 10:04:45AM -0700, Kuniyuki Iwashima wrote: > > From: Breno Leitao <leitao@debian.org> > > Date: Wed, 19 Jul 2023 01:44:12 -0700 > > > Looking at sk_getsockopt function, it is unclear why 128 is a magical > > > number. > > > > > > Use the proper macro, so it becomes clear to understand what the value > > > mean, and get a reference where it is coming from (user-exported API). > > > > > > Signed-off-by: Breno Leitao <leitao@debian.org> > > > --- > > > net/core/sock.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/net/core/sock.c b/net/core/sock.c > > > index 9370fd50aa2c..58b6f00197d6 100644 > > > --- a/net/core/sock.c > > > +++ b/net/core/sock.c > > > @@ -1815,7 +1815,7 @@ int sk_getsockopt(struct sock *sk, int level, int optname, > > > > > > case SO_PEERNAME: > > > { > > > - char address[128]; > > > + char address[_K_SS_MAXSIZE]; > > > > I guess you saw a bug caught by the fortified memcpy(), but this > > doesn't fix it properly. > > Not really, in fact. I was reading this code, and I found this > discussion a while ago, where I got the idea: > > https://lore.kernel.org/lkml/20140930.005925.995989898229686123.davem@davemloft.net/ I got it, but I prefer using struct sockaddr_storage as done in other places. $ grep -rn sockaddr_storage net/ Also, there would be some situations where we must cast each family-specific address back to sockaddr_storage for fortified library. Then, it makes more sense to use sockaddr_storage rather than _K_SS_MAXSIZE. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: Use _K_SS_MAXSIZE instead of absolute value 2023-07-19 17:30 ` Kuniyuki Iwashima @ 2023-07-20 9:29 ` Breno Leitao 0 siblings, 0 replies; 5+ messages in thread From: Breno Leitao @ 2023-07-20 9:29 UTC (permalink / raw) To: Kuniyuki Iwashima Cc: alexander, ast, davem, dhowells, edumazet, kernelxing, kuba, leit, linux-kernel, lucien.xin, martin.lau, netdev, pabeni On Wed, Jul 19, 2023 at 10:30:17AM -0700, Kuniyuki Iwashima wrote: > From: Breno Leitao <leitao@debian.org> > Date: Wed, 19 Jul 2023 10:18:49 -0700 > > On Wed, Jul 19, 2023 at 10:04:45AM -0700, Kuniyuki Iwashima wrote: > > > From: Breno Leitao <leitao@debian.org> > > > Date: Wed, 19 Jul 2023 01:44:12 -0700 > > > > Looking at sk_getsockopt function, it is unclear why 128 is a magical > > > > number. > > > > > > > > Use the proper macro, so it becomes clear to understand what the value > > > > mean, and get a reference where it is coming from (user-exported API). > > > > > > > > Signed-off-by: Breno Leitao <leitao@debian.org> > > > > --- > > > > net/core/sock.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/net/core/sock.c b/net/core/sock.c > > > > index 9370fd50aa2c..58b6f00197d6 100644 > > > > --- a/net/core/sock.c > > > > +++ b/net/core/sock.c > > > > @@ -1815,7 +1815,7 @@ int sk_getsockopt(struct sock *sk, int level, int optname, > > > > > > > > case SO_PEERNAME: > > > > { > > > > - char address[128]; > > > > + char address[_K_SS_MAXSIZE]; > > > > > > I guess you saw a bug caught by the fortified memcpy(), but this > > > doesn't fix it properly. > > > > Not really, in fact. I was reading this code, and I found this > > discussion a while ago, where I got the idea: > > > > https://lore.kernel.org/lkml/20140930.005925.995989898229686123.davem@davemloft.net/ > > I got it, but I prefer using struct sockaddr_storage as done in > other places. > > $ grep -rn sockaddr_storage net/ > > Also, there would be some situations where we must cast each > family-specific address back to sockaddr_storage for fortified > library. > > Then, it makes more sense to use sockaddr_storage rather than > _K_SS_MAXSIZE. Agree, that is a better fix. Thanks for working on it! ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-07-20 9:32 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-07-19 8:44 [PATCH net-next] net: Use _K_SS_MAXSIZE instead of absolute value Breno Leitao 2023-07-19 17:04 ` Kuniyuki Iwashima 2023-07-19 17:18 ` Breno Leitao 2023-07-19 17:30 ` Kuniyuki Iwashima 2023-07-20 9:29 ` Breno Leitao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).