diff mbox series

lkdtm/bugs: Switch from 1-element array to flexible array

Message ID 20230522212949.never.283-kees@kernel.org
State New
Headers
Series lkdtm/bugs: Switch from 1-element array to flexible array |

Commit Message

Kees Cook May 22, 2023, 9:29 p.m. UTC 
  The testing for ARRAY_BOUNDS just wants an uninstrumented array,
and the proper flexible array definition is fine for that.

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/misc/lkdtm/bugs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Bill Wendling May 22, 2023, 10:38 p.m. UTC | #1 
On Mon, May 22, 2023 at 2:30 PM Kees Cook <keescook@chromium.org> wrote:
>
> The testing for ARRAY_BOUNDS just wants an uninstrumented array,
> and the proper flexible array definition is fine for that.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>

Reviewed-by: Bill Wendling <morbo@google.com>

> ---
>  drivers/misc/lkdtm/bugs.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
> index 48821f4c2b21..224f42cdddf2 100644
> --- a/drivers/misc/lkdtm/bugs.c
> +++ b/drivers/misc/lkdtm/bugs.c
> @@ -305,11 +305,10 @@ static void lkdtm_OVERFLOW_UNSIGNED(void)
>         ignored = value;
>  }
>
> -/* Intentionally using old-style flex array definition of 1 byte. */
>  struct array_bounds_flex_array {
>         int one;
>         int two;
> -       char data[1];
> +       char data[];
>  };
>
>  struct array_bounds {
> --
> 2.34.1
>
kernel test robot May 23, 2023, 1:57 a.m. UTC | #2 
Hi Kees,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus soc/for-next kees/for-next/pstore kees/for-next/kspp linus/master v6.4-rc3 next-20230522]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kees-Cook/lkdtm-bugs-Switch-from-1-element-array-to-flexible-array/20230523-053132
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20230522212949.never.283-kees%40kernel.org
patch subject: [PATCH] lkdtm/bugs: Switch from 1-element array to flexible array
config: hexagon-randconfig-r045-20230522
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project b0fb98227c90adf2536c9ad644a74d5e92961111)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/af7b561141f8723e1c5c3339bdc5e782a62fbcb6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Kees-Cook/lkdtm-bugs-Switch-from-1-element-array-to-flexible-array/20230523-053132
        git checkout af7b561141f8723e1c5c3339bdc5e782a62fbcb6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/misc/lkdtm/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305230934.cRtSUQia-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/misc/lkdtm/bugs.c:343:24: error: invalid application of 'sizeof' to an incomplete type 'char[]'
           for (i = 0; i < sizeof(not_checked->data) + 1; i++)
                                 ^~~~~~~~~~~~~~~~~~~
   1 error generated.


vim +343 drivers/misc/lkdtm/bugs.c

ae2e1aad3e48e4 Kees Cook        2020-04-06  320  
73f62e60d80c2d Kees Cook        2022-03-03  321  static void lkdtm_ARRAY_BOUNDS(void)
ae2e1aad3e48e4 Kees Cook        2020-04-06  322  {
ae2e1aad3e48e4 Kees Cook        2020-04-06  323  	struct array_bounds_flex_array *not_checked;
ae2e1aad3e48e4 Kees Cook        2020-04-06  324  	struct array_bounds *checked;
ae2e1aad3e48e4 Kees Cook        2020-04-06  325  	volatile int i;
ae2e1aad3e48e4 Kees Cook        2020-04-06  326  
ae2e1aad3e48e4 Kees Cook        2020-04-06  327  	not_checked = kmalloc(sizeof(*not_checked) * 2, GFP_KERNEL);
ae2e1aad3e48e4 Kees Cook        2020-04-06  328  	checked = kmalloc(sizeof(*checked) * 2, GFP_KERNEL);
4a9800c81d2f34 Jiasheng Jiang   2022-01-20  329  	if (!not_checked || !checked) {
4a9800c81d2f34 Jiasheng Jiang   2022-01-20  330  		kfree(not_checked);
4a9800c81d2f34 Jiasheng Jiang   2022-01-20  331  		kfree(checked);
4a9800c81d2f34 Jiasheng Jiang   2022-01-20  332  		return;
4a9800c81d2f34 Jiasheng Jiang   2022-01-20  333  	}
ae2e1aad3e48e4 Kees Cook        2020-04-06  334  
ae2e1aad3e48e4 Kees Cook        2020-04-06  335  	pr_info("Array access within bounds ...\n");
ae2e1aad3e48e4 Kees Cook        2020-04-06  336  	/* For both, touch all bytes in the actual member size. */
ae2e1aad3e48e4 Kees Cook        2020-04-06  337  	for (i = 0; i < sizeof(checked->data); i++)
ae2e1aad3e48e4 Kees Cook        2020-04-06  338  		checked->data[i] = 'A';
ae2e1aad3e48e4 Kees Cook        2020-04-06  339  	/*
ae2e1aad3e48e4 Kees Cook        2020-04-06  340  	 * For the uninstrumented flex array member, also touch 1 byte
ae2e1aad3e48e4 Kees Cook        2020-04-06  341  	 * beyond to verify it is correctly uninstrumented.
ae2e1aad3e48e4 Kees Cook        2020-04-06  342  	 */
ae2e1aad3e48e4 Kees Cook        2020-04-06 @343  	for (i = 0; i < sizeof(not_checked->data) + 1; i++)
ae2e1aad3e48e4 Kees Cook        2020-04-06  344  		not_checked->data[i] = 'A';
ae2e1aad3e48e4 Kees Cook        2020-04-06  345  
ae2e1aad3e48e4 Kees Cook        2020-04-06  346  	pr_info("Array access beyond bounds ...\n");
ae2e1aad3e48e4 Kees Cook        2020-04-06  347  	for (i = 0; i < sizeof(checked->data) + 1; i++)
ae2e1aad3e48e4 Kees Cook        2020-04-06  348  		checked->data[i] = 'B';
ae2e1aad3e48e4 Kees Cook        2020-04-06  349  
ae2e1aad3e48e4 Kees Cook        2020-04-06  350  	kfree(not_checked);
ae2e1aad3e48e4 Kees Cook        2020-04-06  351  	kfree(checked);
464e86b4abadfc Kees Cook        2020-06-25  352  	pr_err("FAIL: survived array bounds overflow!\n");
8bfdbddd68249e Christophe Leroy 2022-04-11  353  	if (IS_ENABLED(CONFIG_UBSAN_BOUNDS))
8bfdbddd68249e Christophe Leroy 2022-04-11  354  		pr_expected_config(CONFIG_UBSAN_TRAP);
8bfdbddd68249e Christophe Leroy 2022-04-11  355  	else
c75be56e35b2ee Kees Cook        2021-08-18  356  		pr_expected_config(CONFIG_UBSAN_BOUNDS);
ae2e1aad3e48e4 Kees Cook        2020-04-06  357  }
ae2e1aad3e48e4 Kees Cook        2020-04-06  358
diff mbox series

Patch

diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
index 48821f4c2b21..224f42cdddf2 100644
--- a/drivers/misc/lkdtm/bugs.c
+++ b/drivers/misc/lkdtm/bugs.c
@@ -305,11 +305,10 @@  static void lkdtm_OVERFLOW_UNSIGNED(void)
 	ignored = value;
 }
 
-/* Intentionally using old-style flex array definition of 1 byte. */
 struct array_bounds_flex_array {
 	int one;
 	int two;
-	char data[1];
+	char data[];
 };
 
 struct array_bounds {