linux-kernel - Re: [PATCH] asm-generic/ioctl.h: use BUILD_BUG_ON

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a1b5Tr8Gt_DcUq9JQj4G6O5ZHf44P2ZdYZRGQY8iPs43Q@mail.gmail.com>
Date:   Tue, 23 Feb 2021 21:04:10 +0100
From:   Arnd Bergmann <arnd@...nel.org>
To:     Masahiro Yamada <masahiroy@...nel.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux-arch <linux-arch@...r.kernel.org>
Subject: Re: [PATCH] asm-generic/ioctl.h: use BUILD_BUG_ON_ZERO() for type check

On Tue, Feb 23, 2021 at 11:06 AM Masahiro Yamada <masahiroy@...nel.org> wrote:

>
> -#ifdef __CHECKER__
> -#define _IOC_TYPECHECK(t) (sizeof(t))
> -#else
>  /* provoke compile error for invalid uses of size argument */
> -extern unsigned int __invalid_size_argument_for_IOC;
> +#undef _IOC_TYPECHECK
>  #define _IOC_TYPECHECK(t) \
> -       ((sizeof(t) == sizeof(t[1]) && \
> -         sizeof(t) < (1 << _IOC_SIZEBITS)) ? \
> -         sizeof(t) : __invalid_size_argument_for_IOC)
> -#endif
> +       BUILD_BUG_ON_ZERO(sizeof(t) != sizeof(t[1]) || \
> +                         sizeof(t) >= (1 << _IOC_SIZEBITS))

Using BUILD_BUG_ON_ZERO sounds like a good idea

>  #endif /* _ASM_GENERIC_IOCTL_H */
> diff --git a/include/uapi/asm-generic/ioctl.h b/include/uapi/asm-generic/ioctl.h
> index a84f4db8a250..d50bd39ec3e3 100644
> --- a/include/uapi/asm-generic/ioctl.h
> +++ b/include/uapi/asm-generic/ioctl.h
> @@ -72,9 +72,8 @@
>          ((nr)   << _IOC_NRSHIFT) | \
>          ((size) << _IOC_SIZESHIFT))
>
> -#ifndef __KERNEL__
> -#define _IOC_TYPECHECK(t) (sizeof(t))
> -#endif
> +#define _IOC_TYPECHECK(t)      0
> +#define _IOC_SIZE_WITH_TYPECHECK(t)    (sizeof(t) + _IOC_TYPECHECK(t))

But I think replacing the #ifndef with an #undef in the other file makes it
harder to understand when reading through it and trying to understand
what it would do when this gets included from kernel and user space.

        Arnd