Re: [LTP] [PATCH] syscalls/sockioctl: Align buffer to struct ifreq

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> In setup3, the following line can lead to an undefined behavior:
>   ifr = *(struct ifreq *)ifc.ifc_buf;
> 
> Indeed, at this point it can be assumed that ifc.ifc_buf is suitably
> aligned for struct ifreq.
> However, ifc.ifc_buf is assigned to buf which has no alignment
> constraints. This means there exists cases where buf is not suitably
> aligned to load a struct ifreq, which can generate a SIGBUS.
> 
> Force the alignment of buf to that of struct ifreq.
> 
> Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
> ---
> CI Build: https://github.com/Teo-CD/ltp/actions/runs/4502204132
> 
>  testcases/kernel/syscalls/sockioctl/sockioctl01.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/syscalls/sockioctl/sockioctl01.c b/testcases/kernel/syscalls/sockioctl/sockioctl01.c
> index 486236af9d6b..e63aa1921877 100644
> --- a/testcases/kernel/syscalls/sockioctl/sockioctl01.c
> +++ b/testcases/kernel/syscalls/sockioctl/sockioctl01.c
> @@ -52,7 +52,13 @@ static struct ifreq ifr;
>  static int sinlen;
>  static int optval;
>  
> -static char buf[8192];
> +/*
> + * buf has no alignment constraints by default. However, it is used to load
> + * a struct ifreq in setup3, which requires it to have an appropriate alignment
> + * to prevent a possible undefined behavior.
> + */
> +static char buf[8192]
> +	__attribute__((aligned(__alignof__(struct ifreq))));
>  
>  static void setup(void);
>  static void setup0(void);

Looking at the code, shouldn't we rather than that declare the buffer as
an struct ifreq array, that would naturally align the buffer without any
need for tricky __attribute__.

diff --git a/testcases/kernel/syscalls/sockioctl/sockioctl01.c b/testcases/kernel/syscalls/sockioctl/sockioctl01.c
index 51dac9c16..206a4999e 100644
--- a/testcases/kernel/syscalls/sockioctl/sockioctl01.c
+++ b/testcases/kernel/syscalls/sockioctl/sockioctl01.c
@@ -52,7 +52,7 @@ static struct ifreq ifr;
 static int sinlen;
 static int optval;

-static char buf[8192];
+static struct ifreq buf[200];

 static void setup(void);
 static void setup0(void);
@@ -218,7 +218,7 @@ static void setup2(void)
        s = SAFE_SOCKET(cleanup, tdat[testno].domain, tdat[testno].type,
                        tdat[testno].proto);
        ifc.ifc_len = sizeof(buf);
-       ifc.ifc_buf = buf;
+       ifc.ifc_buf = (void*)buf;
 }


-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp