All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH][next] ceph: Replace zero-length array with flexible array member
@ 2021-05-14 21:52 Gustavo A. R. Silva
  2021-05-15 10:42 ` Ilya Dryomov
  0 siblings, 1 reply; 3+ messages in thread
From: Gustavo A. R. Silva @ 2021-05-14 21:52 UTC (permalink / raw)
  To: Ilya Dryomov, Jeff Layton, David S. Miller, Jakub Kicinski
  Cc: ceph-devel, netdev, linux-kernel, Gustavo A. R. Silva,
	linux-hardening

There is a regular need in the kernel to provide a way to declare
having a dynamically sized set of trailing elements in a structure.
Kernel code should always use “flexible array members”[1] for these
cases. The older style of one-element or zero-length arrays should
no longer be used[2].

Notice that, in this case, sizeof(au->reply_buf) translates to zero,
becase in the original code reply_buf is a zero-length array. Now that
reply_buf is transformed into a flexible array, the mentioned line of
code is now replaced by a literal 0.

Also, as a safeguard, explicitly assign NULL to
auth->authorizer_reply_buf, as no heap is allocated for it, therefore
it should not be accessible.

[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2] https://www.kernel.org/doc/html/v5.10/process/deprecated.html#zero-length-and-one-element-arrays

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 net/ceph/auth_none.c | 4 ++--
 net/ceph/auth_none.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ceph/auth_none.c b/net/ceph/auth_none.c
index 70e86e462250..10ee16d2cbf0 100644
--- a/net/ceph/auth_none.c
+++ b/net/ceph/auth_none.c
@@ -111,8 +111,8 @@ static int ceph_auth_none_create_authorizer(
 	auth->authorizer = (struct ceph_authorizer *) au;
 	auth->authorizer_buf = au->buf;
 	auth->authorizer_buf_len = au->buf_len;
-	auth->authorizer_reply_buf = au->reply_buf;
-	auth->authorizer_reply_buf_len = sizeof (au->reply_buf);
+	auth->authorizer_reply_buf_len = 0;
+	auth->authorizer_reply_buf = NULL;
 
 	return 0;
 }
diff --git a/net/ceph/auth_none.h b/net/ceph/auth_none.h
index 4158f064302e..3c68c0ee3dab 100644
--- a/net/ceph/auth_none.h
+++ b/net/ceph/auth_none.h
@@ -16,7 +16,7 @@ struct ceph_none_authorizer {
 	struct ceph_authorizer base;
 	char buf[128];
 	int buf_len;
-	char reply_buf[0];
+	char reply_buf[];
 };
 
 struct ceph_auth_none_info {
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH][next] ceph: Replace zero-length array with flexible array member
  2021-05-14 21:52 [PATCH][next] ceph: Replace zero-length array with flexible array member Gustavo A. R. Silva
@ 2021-05-15 10:42 ` Ilya Dryomov
  2021-05-15 20:50   ` Gustavo A. R. Silva
  0 siblings, 1 reply; 3+ messages in thread
From: Ilya Dryomov @ 2021-05-15 10:42 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Jeff Layton, David S. Miller, Jakub Kicinski, Ceph Development,
	netdev, LKML, linux-hardening

On Fri, May 14, 2021 at 11:51 PM Gustavo A. R. Silva
<gustavoars@kernel.org> wrote:
>
> There is a regular need in the kernel to provide a way to declare
> having a dynamically sized set of trailing elements in a structure.
> Kernel code should always use “flexible array members”[1] for these
> cases. The older style of one-element or zero-length arrays should
> no longer be used[2].
>
> Notice that, in this case, sizeof(au->reply_buf) translates to zero,
> becase in the original code reply_buf is a zero-length array. Now that
> reply_buf is transformed into a flexible array, the mentioned line of
> code is now replaced by a literal 0.
>
> Also, as a safeguard, explicitly assign NULL to
> auth->authorizer_reply_buf, as no heap is allocated for it, therefore
> it should not be accessible.
>
> [1] https://en.wikipedia.org/wiki/Flexible_array_member
> [2] https://www.kernel.org/doc/html/v5.10/process/deprecated.html#zero-length-and-one-element-arrays
>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  net/ceph/auth_none.c | 4 ++--
>  net/ceph/auth_none.h | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/ceph/auth_none.c b/net/ceph/auth_none.c
> index 70e86e462250..10ee16d2cbf0 100644
> --- a/net/ceph/auth_none.c
> +++ b/net/ceph/auth_none.c
> @@ -111,8 +111,8 @@ static int ceph_auth_none_create_authorizer(
>         auth->authorizer = (struct ceph_authorizer *) au;
>         auth->authorizer_buf = au->buf;
>         auth->authorizer_buf_len = au->buf_len;
> -       auth->authorizer_reply_buf = au->reply_buf;
> -       auth->authorizer_reply_buf_len = sizeof (au->reply_buf);
> +       auth->authorizer_reply_buf_len = 0;
> +       auth->authorizer_reply_buf = NULL;
>
>         return 0;
>  }
> diff --git a/net/ceph/auth_none.h b/net/ceph/auth_none.h
> index 4158f064302e..3c68c0ee3dab 100644
> --- a/net/ceph/auth_none.h
> +++ b/net/ceph/auth_none.h
> @@ -16,7 +16,7 @@ struct ceph_none_authorizer {
>         struct ceph_authorizer base;
>         char buf[128];
>         int buf_len;
> -       char reply_buf[0];
> +       char reply_buf[];
>  };
>
>  struct ceph_auth_none_info {

Hi Gustavo,

I went ahead and removed reply_buf.  We never receive authorizer
replies in auth_none mode, so patching it to be a flexible array
is rather pointless.

Thanks,

                Ilya

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][next] ceph: Replace zero-length array with flexible array member
  2021-05-15 10:42 ` Ilya Dryomov
@ 2021-05-15 20:50   ` Gustavo A. R. Silva
  0 siblings, 0 replies; 3+ messages in thread
From: Gustavo A. R. Silva @ 2021-05-15 20:50 UTC (permalink / raw)
  To: Ilya Dryomov, Gustavo A. R. Silva
  Cc: Jeff Layton, David S. Miller, Jakub Kicinski, Ceph Development,
	netdev, LKML, linux-hardening



On 5/15/21 05:42, Ilya Dryomov wrote:
> 
> Hi Gustavo,
> 
> I went ahead and removed reply_buf.  We never receive authorizer
> replies in auth_none mode, so patching it to be a flexible array
> is rather pointless.

Sounds great. :)

Thanks, Ilya.
--
Gustavo

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-05-15 21:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-05-14 21:52 [PATCH][next] ceph: Replace zero-length array with flexible array member Gustavo A. R. Silva
2021-05-15 10:42 ` Ilya Dryomov
2021-05-15 20:50   ` Gustavo A. R. Silva

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.