diff mbox series

[v2] input/vmmouse: Fix device name copies

Message ID 20240104050605.1773158-1-zack.rusin@broadcom.com
State New
Headers
Series [v2] input/vmmouse: Fix device name copies |

Commit Message

Zack Rusin Jan. 4, 2024, 5:06 a.m. UTC 
  Make sure vmmouse_data::phys can hold serio::phys (which is 32 bytes)
plus an extra string, extend it to 64.

Fixes gcc13 warnings:
drivers/input/mouse/vmmouse.c: In function ‘vmmouse_init’:
drivers/input/mouse/vmmouse.c:455:53: warning: ‘/input1’ directive output may be truncated writing 7 bytes into a region of size between 1 and 32 [-Wformat-truncation=]
  455 |         snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
      |                                                     ^~~~~~~
drivers/input/mouse/vmmouse.c:455:9: note: ‘snprintf’ output between 8 and 39 bytes into a destination of size 32
  455 |         snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  456 |                  psmouse->ps2dev.serio->phys);
      |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

v2: Use the exact size for the vmmouse_data::phys

Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
Fixes: 8b8be51b4fd3 ("Input: add vmmouse driver")
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Raul Rangel <rrangel@chromium.org>
Cc: linux-input@vger.kernel.org
Cc: <stable@vger.kernel.org> # v4.1+
---
 drivers/input/mouse/vmmouse.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Zack Rusin Jan. 4, 2024, 5:07 a.m. UTC | #1 
On Thu, Jan 4, 2024 at 12:06 AM Zack Rusin <zack.rusin@broadcom.com> wrote:
>
> Make sure vmmouse_data::phys can hold serio::phys (which is 32 bytes)
> plus an extra string, extend it to 64.
>
> Fixes gcc13 warnings:
> drivers/input/mouse/vmmouse.c: In function ‘vmmouse_init’:
> drivers/input/mouse/vmmouse.c:455:53: warning: ‘/input1’ directive output may be truncated writing 7 bytes into a region of size between 1 and 32 [-Wformat-truncation=]
>   455 |         snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
>       |                                                     ^~~~~~~
> drivers/input/mouse/vmmouse.c:455:9: note: ‘snprintf’ output between 8 and 39 bytes into a destination of size 32
>   455 |         snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   456 |                  psmouse->ps2dev.serio->phys);
>       |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> v2: Use the exact size for the vmmouse_data::phys
>
> Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
> Fixes: 8b8be51b4fd3 ("Input: add vmmouse driver")
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Robert Jarzmik <robert.jarzmik@free.fr>
> Cc: Raul Rangel <rrangel@chromium.org>
> Cc: linux-input@vger.kernel.org
> Cc: <stable@vger.kernel.org> # v4.1+
> ---
>  drivers/input/mouse/vmmouse.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
> index ea9eff7c8099..74131673e2f3 100644
> --- a/drivers/input/mouse/vmmouse.c
> +++ b/drivers/input/mouse/vmmouse.c
> @@ -63,6 +63,8 @@
>  #define VMMOUSE_VENDOR "VMware"
>  #define VMMOUSE_NAME   "VMMouse"
>
> +#define VMMOUSE_PHYS_NAME_POSTFIX_STR "/input1"
> +
>  /**
>   * struct vmmouse_data - private data structure for the vmmouse driver
>   *
> @@ -72,7 +74,8 @@
>   */
>  struct vmmouse_data {
>         struct input_dev *abs_dev;
> -       char phys[32];
> +       char phys[sizeof_field(struct serio, phys) +
> +                 strlen(VMMOUSE_PHYS_NAME_POSTFIX_STR)];
>         char dev_name[128];
>  };
>
> @@ -452,7 +455,8 @@ int vmmouse_init(struct psmouse *psmouse)
>         psmouse->private = priv;
>
>         /* Set up and register absolute device */
> -       snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
> +       snprintf(priv->phys, sizeof(priv->phys),
> +                "%s" VMMOUSE_PHYS_NAME_POSTFIX_STR,
>                  psmouse->ps2dev.serio->phys);
>
>         /* Mimic name setup for relative device in psmouse-base.c */

Sorry, I missed the original discussion of this during the
VMware->Broadcom email transition. How about we just use the exact
sizing then like in the v2?

z
kernel test robot Jan. 4, 2024, 4:03 p.m. UTC | #2 
Hi Zack,

kernel test robot noticed the following build errors:

[auto build test ERROR on dtor-input/next]
[also build test ERROR on dtor-input/for-linus linus/master v6.7-rc8 next-20240104]
[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/Zack-Rusin/input-vmmouse-Fix-device-name-copies/20240104-130724
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link:    https://lore.kernel.org/r/20240104050605.1773158-1-zack.rusin%40broadcom.com
patch subject: [PATCH v2] input/vmmouse: Fix device name copies
config: i386-buildonly-randconfig-001-20240104 (https://download.01.org/0day-ci/archive/20240104/202401042305.WdnDeo57-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240104/202401042305.WdnDeo57-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401042305.WdnDeo57-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/input/mouse/vmmouse.c:77:7: error: variably modified 'phys' at file scope
     char phys[sizeof_field(struct serio, phys) +
          ^~~~


vim +/phys +77 drivers/input/mouse/vmmouse.c

    67	
    68	/**
    69	 * struct vmmouse_data - private data structure for the vmmouse driver
    70	 *
    71	 * @abs_dev: "Absolute" device used to report absolute mouse movement.
    72	 * @phys: Physical path for the absolute device.
    73	 * @dev_name: Name attribute name for the absolute device.
    74	 */
    75	struct vmmouse_data {
    76		struct input_dev *abs_dev;
  > 77		char phys[sizeof_field(struct serio, phys) +
    78			  strlen(VMMOUSE_PHYS_NAME_POSTFIX_STR)];
    79		char dev_name[128];
    80	};
    81
kernel test robot Jan. 4, 2024, 7:23 p.m. UTC | #3 
Hi Zack,

kernel test robot noticed the following build warnings:

[auto build test WARNING on dtor-input/next]
[also build test WARNING on dtor-input/for-linus linus/master v6.7-rc8 next-20240104]
[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/Zack-Rusin/input-vmmouse-Fix-device-name-copies/20240104-130724
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link:    https://lore.kernel.org/r/20240104050605.1773158-1-zack.rusin%40broadcom.com
patch subject: [PATCH v2] input/vmmouse: Fix device name copies
config: i386-randconfig-061-20240104 (https://download.01.org/0day-ci/archive/20240105/202401050307.tWbIHuS1-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240105/202401050307.tWbIHuS1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401050307.tWbIHuS1-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/input/mouse/vmmouse.c:77:52: sparse: sparse: Variable length array is used.
>> drivers/input/mouse/vmmouse.c:77:14: sparse: sparse: flexible array member 'phys' is not last

vim +77 drivers/input/mouse/vmmouse.c

    67	
    68	/**
    69	 * struct vmmouse_data - private data structure for the vmmouse driver
    70	 *
    71	 * @abs_dev: "Absolute" device used to report absolute mouse movement.
    72	 * @phys: Physical path for the absolute device.
    73	 * @dev_name: Name attribute name for the absolute device.
    74	 */
    75	struct vmmouse_data {
    76		struct input_dev *abs_dev;
  > 77		char phys[sizeof_field(struct serio, phys) +
    78			  strlen(VMMOUSE_PHYS_NAME_POSTFIX_STR)];
    79		char dev_name[128];
    80	};
    81
Zack Rusin Jan. 4, 2024, 9:29 p.m. UTC | #4 
On Thu, Jan 4, 2024 at 11:04 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Zack,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on dtor-input/next]
> [also build test ERROR on dtor-input/for-linus linus/master v6.7-rc8 next-20240104]
> [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/Zack-Rusin/input-vmmouse-Fix-device-name-copies/20240104-130724
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
> patch link:    https://lore.kernel.org/r/20240104050605.1773158-1-zack.rusin%40broadcom.com
> patch subject: [PATCH v2] input/vmmouse: Fix device name copies
> config: i386-buildonly-randconfig-001-20240104 (https://download.01.org/0day-ci/archive/20240104/202401042305.WdnDeo57-lkp@intel.com/config)
> compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240104/202401042305.WdnDeo57-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202401042305.WdnDeo57-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> >> drivers/input/mouse/vmmouse.c:77:7: error: variably modified 'phys' at file scope
>      char phys[sizeof_field(struct serio, phys) +
>           ^~~~

This can be trivially fixed by using __builtin_strlen which with
fortify-strings.h is used anyway but before respining this I'll wait
to hear if this approach is ok. The sparse warning we can't do much
about because it doesn't recognize the fact that the array size is a
compile time expression.

z
kernel test robot Jan. 4, 2024, 10:01 p.m. UTC | #5 
Hi Zack,

kernel test robot noticed the following build errors:

[auto build test ERROR on dtor-input/next]
[also build test ERROR on dtor-input/for-linus linus/master v6.7-rc8 next-20240104]
[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/Zack-Rusin/input-vmmouse-Fix-device-name-copies/20240104-130724
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link:    https://lore.kernel.org/r/20240104050605.1773158-1-zack.rusin%40broadcom.com
patch subject: [PATCH v2] input/vmmouse: Fix device name copies
config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20240105/202401050506.N1aMF9sD-lkp@intel.com/config)
compiler: ClangBuiltLinux clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240105/202401050506.N1aMF9sD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401050506.N1aMF9sD-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> drivers/input/mouse/vmmouse.c:78:5: warning: variable length array used [-Wvla]
      77 |         char phys[sizeof_field(struct serio, phys) +
         |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      78 |                   strlen(VMMOUSE_PHYS_NAME_POSTFIX_STR)];
         |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/input/mouse/vmmouse.c:77:7: error: fields must have a constant size: 'variable length array in structure' extension will never be supported
      77 |         char phys[sizeof_field(struct serio, phys) +
         |              ^
   1 warning and 1 error generated.


vim +77 drivers/input/mouse/vmmouse.c

    67	
    68	/**
    69	 * struct vmmouse_data - private data structure for the vmmouse driver
    70	 *
    71	 * @abs_dev: "Absolute" device used to report absolute mouse movement.
    72	 * @phys: Physical path for the absolute device.
    73	 * @dev_name: Name attribute name for the absolute device.
    74	 */
    75	struct vmmouse_data {
    76		struct input_dev *abs_dev;
  > 77		char phys[sizeof_field(struct serio, phys) +
  > 78			  strlen(VMMOUSE_PHYS_NAME_POSTFIX_STR)];
    79		char dev_name[128];
    80	};
    81
David Laight Jan. 4, 2024, 10:57 p.m. UTC | #6 
From: Zack Rusin
> Sent: 04 January 2024 05:06
> 
> Make sure vmmouse_data::phys can hold serio::phys (which is 32 bytes)
> plus an extra string, extend it to 64.
> 
> Fixes gcc13 warnings:
> drivers/input/mouse/vmmouse.c: In function ‘vmmouse_init’:
> drivers/input/mouse/vmmouse.c:455:53: warning: ‘/input1’ directive output may be truncated writing 7
> bytes into a region of size between 1 and 32 [-Wformat-truncation=]
>   455 |         snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
>       |                                                     ^~~~~~~
> drivers/input/mouse/vmmouse.c:455:9: note: ‘snprintf’ output between 8 and 39 bytes into a destination
> of size 32
>   455 |         snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   456 |                  psmouse->ps2dev.serio->phys);
>       |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> v2: Use the exact size for the vmmouse_data::phys
> 
> Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
> Fixes: 8b8be51b4fd3 ("Input: add vmmouse driver")
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Robert Jarzmik <robert.jarzmik@free.fr>
> Cc: Raul Rangel <rrangel@chromium.org>
> Cc: linux-input@vger.kernel.org
> Cc: <stable@vger.kernel.org> # v4.1+
> ---
>  drivers/input/mouse/vmmouse.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
> index ea9eff7c8099..74131673e2f3 100644
> --- a/drivers/input/mouse/vmmouse.c
> +++ b/drivers/input/mouse/vmmouse.c
> @@ -63,6 +63,8 @@
>  #define VMMOUSE_VENDOR "VMware"
>  #define VMMOUSE_NAME   "VMMouse"
> 
> +#define VMMOUSE_PHYS_NAME_POSTFIX_STR "/input1"
> +
>  /**
>   * struct vmmouse_data - private data structure for the vmmouse driver
>   *
> @@ -72,7 +74,8 @@
>   */
>  struct vmmouse_data {
>  	struct input_dev *abs_dev;
> -	char phys[32];
> +	char phys[sizeof_field(struct serio, phys) +
> +		  strlen(VMMOUSE_PHYS_NAME_POSTFIX_STR)];
>  	char dev_name[128];
>  };
> 
> @@ -452,7 +455,8 @@ int vmmouse_init(struct psmouse *psmouse)
>  	psmouse->private = priv;
> 
>  	/* Set up and register absolute device */
> -	snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
> +	snprintf(priv->phys, sizeof(priv->phys),
> +		 "%s" VMMOUSE_PHYS_NAME_POSTFIX_STR,
>  		 psmouse->ps2dev.serio->phys);

Notwithstanding any error (fixed) or not from the gcc/build robert
or sparse that 'fix' is entirely horrible.

Related I'm guessing that dev_name[128] is either likely to
be truncated or massively far too long?

There are a few way to get gcc to STFU :-)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
diff mbox series

Patch

diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
index ea9eff7c8099..74131673e2f3 100644
--- a/drivers/input/mouse/vmmouse.c
+++ b/drivers/input/mouse/vmmouse.c
@@ -63,6 +63,8 @@ 
 #define VMMOUSE_VENDOR "VMware"
 #define VMMOUSE_NAME   "VMMouse"
 
+#define VMMOUSE_PHYS_NAME_POSTFIX_STR "/input1"
+
 /**
  * struct vmmouse_data - private data structure for the vmmouse driver
  *
@@ -72,7 +74,8 @@ 
  */
 struct vmmouse_data {
 	struct input_dev *abs_dev;
-	char phys[32];
+	char phys[sizeof_field(struct serio, phys) +
+		  strlen(VMMOUSE_PHYS_NAME_POSTFIX_STR)];
 	char dev_name[128];
 };
 
@@ -452,7 +455,8 @@  int vmmouse_init(struct psmouse *psmouse)
 	psmouse->private = priv;
 
 	/* Set up and register absolute device */
-	snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
+	snprintf(priv->phys, sizeof(priv->phys),
+		 "%s" VMMOUSE_PHYS_NAME_POSTFIX_STR,
 		 psmouse->ps2dev.serio->phys);
 
 	/* Mimic name setup for relative device in psmouse-base.c */