2023-12-03 06:08:09

by kernel test robot

[permalink] [raw]
Subject: drivers/input/touchscreen/egalax_ts_serial.c:116:21: warning: '/input0' directive output may be truncated writing 7 bytes into a region of size between 1 and 32

Hi B?sz?rm?nyi,

FYI, the error/warning still remains.

tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 815fb87b753055df2d9e50f6cd80eb10235fe3e9
commit: 6b0f8f9c52efe24d6dac06ab963b7bd91c723751 Input: add eGalaxTouch serial touchscreen driver
date: 8 years ago
config: x86_64-randconfig-r032-20230515 (https://download.01.org/0day-ci/archive/20231202/[email protected]/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/20231202/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

In file included from include/linux/kobject.h:21,
from include/linux/module.h:17,
from drivers/input/touchscreen/egalax_ts_serial.c:19:
include/linux/sysfs.h: In function 'sysfs_get_dirent':
include/linux/sysfs.h:517:44: warning: pointer targets in passing argument 2 of 'kernfs_find_and_get' differ in signedness [-Wpointer-sign]
517 | return kernfs_find_and_get(parent, name);
| ^~~~
| |
| const unsigned char *
In file included from include/linux/sysfs.h:15:
include/linux/kernfs.h:428:57: note: expected 'const char *' but argument is of type 'const unsigned char *'
428 | kernfs_find_and_get(struct kernfs_node *kn, const char *name)
| ~~~~~~~~~~~~^~~~
drivers/input/touchscreen/egalax_ts_serial.c: In function 'egalax_connect':
>> drivers/input/touchscreen/egalax_ts_serial.c:116:21: warning: '/input0' directive output may be truncated writing 7 bytes into a region of size between 1 and 32 [-Wformat-truncation=]
116 | "%s/input0", serio->phys);
| ^~~~~~~
drivers/input/touchscreen/egalax_ts_serial.c:115:9: note: 'snprintf' output between 8 and 39 bytes into a destination of size 32
115 | snprintf(egalax->phys, sizeof(egalax->phys),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
116 | "%s/input0", serio->phys);
| ~~~~~~~~~~~~~~~~~~~~~~~~~


vim +116 drivers/input/touchscreen/egalax_ts_serial.c

94
95 /*
96 * egalax_connect() is the routine that is called when someone adds a
97 * new serio device that supports egalax protocol and registers it as
98 * an input device. This is usually accomplished using inputattach.
99 */
100 static int egalax_connect(struct serio *serio, struct serio_driver *drv)
101 {
102 struct egalax *egalax;
103 struct input_dev *input_dev;
104 int error;
105
106 egalax = kzalloc(sizeof(struct egalax), GFP_KERNEL);
107 input_dev = input_allocate_device();
108 if (!egalax) {
109 error = -ENOMEM;
110 goto err_free_mem;
111 }
112
113 egalax->serio = serio;
114 egalax->input = input_dev;
115 snprintf(egalax->phys, sizeof(egalax->phys),
> 116 "%s/input0", serio->phys);
117
118 input_dev->name = "EETI eGalaxTouch Serial TouchScreen";
119 input_dev->phys = egalax->phys;
120 input_dev->id.bustype = BUS_RS232;
121 input_dev->id.vendor = SERIO_EGALAX;
122 input_dev->id.product = 0;
123 input_dev->id.version = 0x0001;
124 input_dev->dev.parent = &serio->dev;
125
126 input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
127 input_set_abs_params(input_dev, ABS_X,
128 EGALAX_MIN_XC, EGALAX_MAX_XC, 0, 0);
129 input_set_abs_params(input_dev, ABS_Y,
130 EGALAX_MIN_YC, EGALAX_MAX_YC, 0, 0);
131
132 serio_set_drvdata(serio, egalax);
133
134 error = serio_open(serio, drv);
135 if (error)
136 goto err_reset_drvdata;
137
138 error = input_register_device(input_dev);
139 if (error)
140 goto err_close_serio;
141
142 return 0;
143
144 err_close_serio:
145 serio_close(serio);
146 err_reset_drvdata:
147 serio_set_drvdata(serio, NULL);
148 err_free_mem:
149 input_free_device(input_dev);
150 kfree(egalax);
151 return error;
152 }
153

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


2023-12-03 06:23:27

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: drivers/input/touchscreen/egalax_ts_serial.c:116:21: warning: '/input0' directive output may be truncated writing 7 bytes into a region of size between 1 and 32

On Sun, Dec 03, 2023 at 02:07:07PM +0800, kernel test robot wrote:
> Hi B?sz?rm?nyi,
>
> FYI, the error/warning still remains.
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 815fb87b753055df2d9e50f6cd80eb10235fe3e9
> commit: 6b0f8f9c52efe24d6dac06ab963b7bd91c723751 Input: add eGalaxTouch serial touchscreen driver
> date: 8 years ago
> config: x86_64-randconfig-r032-20230515 (https://download.01.org/0day-ci/archive/20231202/[email protected]/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/20231202/[email protected]/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 <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All warnings (new ones prefixed by >>):
>
> In file included from include/linux/kobject.h:21,
> from include/linux/module.h:17,
> from drivers/input/touchscreen/egalax_ts_serial.c:19:
> include/linux/sysfs.h: In function 'sysfs_get_dirent':
> include/linux/sysfs.h:517:44: warning: pointer targets in passing argument 2 of 'kernfs_find_and_get' differ in signedness [-Wpointer-sign]
> 517 | return kernfs_find_and_get(parent, name);
> | ^~~~
> | |
> | const unsigned char *
> In file included from include/linux/sysfs.h:15:
> include/linux/kernfs.h:428:57: note: expected 'const char *' but argument is of type 'const unsigned char *'
> 428 | kernfs_find_and_get(struct kernfs_node *kn, const char *name)
> | ~~~~~~~~~~~~^~~~
> drivers/input/touchscreen/egalax_ts_serial.c: In function 'egalax_connect':
> >> drivers/input/touchscreen/egalax_ts_serial.c:116:21: warning: '/input0' directive output may be truncated writing 7 bytes into a region of size between 1 and 32 [-Wformat-truncation=]
> 116 | "%s/input0", serio->phys);

Please stop using -Wformat-truncation and report warnings as errors.
This particular warning is disabled in normal builds and the truncation
is desired outcome here.

Thanks.

--
Dmitry

2023-12-03 09:06:57

by Zoltán Böszörményi

[permalink] [raw]
Subject: [PATCH] egalax_ts_serial: Fix potential buffer overflow

Update my old (defunct) email addresses in passing.

Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Signed-off-by: Zoltán Böszörményi <[email protected]>
---
CREDITS | 3 ++-
drivers/input/touchscreen/egalax_ts_serial.c | 6 +++---
2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/CREDITS b/CREDITS
index f33a33fd2371..5efccb40d577 100644
--- a/CREDITS
+++ b/CREDITS
@@ -470,8 +470,9 @@ S: Montreal, Quebec
S: Canada

N: Zoltán Böszörményi
-E: [email protected]
+E: [email protected]
D: MTRR emulation with Cyrix style ARR registers, Athlon MTRR support
+D: eGalax serial touchscreen support

N: John Boyd
E: [email protected]
diff --git a/drivers/input/touchscreen/egalax_ts_serial.c b/drivers/input/touchscreen/egalax_ts_serial.c
index 375922d3a6d1..f8b56896a42f 100644
--- a/drivers/input/touchscreen/egalax_ts_serial.c
+++ b/drivers/input/touchscreen/egalax_ts_serial.c
@@ -2,7 +2,7 @@
/*
* EETI Egalax serial touchscreen driver
*
- * Copyright (c) 2015 Zoltán Böszörményi <[email protected]>
+ * Copyright (c) 2015 Zoltán Böszörményi <[email protected]>
*
* based on the
*
@@ -42,7 +42,7 @@ struct egalax {
struct serio *serio;
int idx;
u8 data[EGALAX_FORMAT_MAX_LENGTH];
- char phys[32];
+ char phys[NAME_MAX];
};

static void egalax_process_data(struct egalax *egalax)
@@ -185,6 +185,6 @@ static struct serio_driver egalax_drv = {
};
module_serio_driver(egalax_drv);

-MODULE_AUTHOR("Zoltán Böszörményi <[email protected]>");
+MODULE_AUTHOR("Zoltán Böszörményi <[email protected]>");
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE("GPL v2");
--
2.42.0

2023-12-03 18:35:32

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] egalax_ts_serial: Fix potential buffer overflow

Hi Zolt?n,

On Sun, Dec 03, 2023 at 10:06:00AM +0100, Zolt?n B?sz?rm?nyi wrote:
> @@ -42,7 +42,7 @@ struct egalax {
> struct serio *serio;
> int idx;
> u8 data[EGALAX_FORMAT_MAX_LENGTH];
> - char phys[32];
> + char phys[NAME_MAX];

This simply wastes 200+ bytes for no good reason. It is perfectly fine
to truncate phys string (which does not happen in practice).

If you feel strongly about it then maybe use devm_kasprintf() to
allocate the needed buffer.

Thanks.

--
Dmitry

2023-12-04 06:38:49

by Zoltán Böszörményi

[permalink] [raw]
Subject: Re: [PATCH] egalax_ts_serial: Fix potential buffer overflow

2023. 12. 03. 19:35 keltezéssel, Dmitry Torokhov írta:
> Hi Zoltán,
>
> On Sun, Dec 03, 2023 at 10:06:00AM +0100, Zoltán Böszörményi wrote:
>> @@ -42,7 +42,7 @@ struct egalax {
>> struct serio *serio;
>> int idx;
>> u8 data[EGALAX_FORMAT_MAX_LENGTH];
>> - char phys[32];
>> + char phys[NAME_MAX];
> This simply wastes 200+ bytes for no good reason. It is perfectly fine
> to truncate phys string (which does not happen in practice).

Okay, I modified the phys[] array to just be 40 bytes.
That's not that wasteful and still avoids the warning.

I noticed that other TS drivers emit the same warning.

>
> If you feel strongly about it then maybe use devm_kasprintf() to
> allocate the needed buffer.
>
> Thanks.
>

2023-12-04 06:41:07

by Zoltán Böszörményi

[permalink] [raw]
Subject: [PATCH v2] egalax_ts_serial: Fix potential buffer overflow

Increase phys[] array size to 40 bytes to avoid
this warning:

CC [M] drivers/input/touchscreen/egalax_ts_serial.o
drivers/input/touchscreen/egalax_ts_serial.c: In function ‘egalax_connect’:
drivers/input/touchscreen/egalax_ts_serial.c:112:21: warning: ‘/input0’ directive output may be truncated writing 7 bytes into a region of size between 1 and 32 [-Wformat-truncation=]
112 | "%s/input0", serio->phys);
| ^~~~~~~
drivers/input/touchscreen/egalax_ts_serial.c:111:9: note: ‘snprintf’ output between 8 and 39 bytes into a destination of size 32
111 | snprintf(egalax->phys, sizeof(egalax->phys),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
112 | "%s/input0", serio->phys);
| ~~~~~~~~~~~~~~~~~~~~~~~~~

Update my old (defunct) email addresses in passing.

Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Signed-off-by: Zoltán Böszörményi <[email protected]>
---
CREDITS | 3 ++-
drivers/input/touchscreen/egalax_ts_serial.c | 6 +++---
2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/CREDITS b/CREDITS
index f33a33fd2371..5efccb40d577 100644
--- a/CREDITS
+++ b/CREDITS
@@ -470,8 +470,9 @@ S: Montreal, Quebec
S: Canada

N: Zoltán Böszörményi
-E: [email protected]
+E: [email protected]
D: MTRR emulation with Cyrix style ARR registers, Athlon MTRR support
+D: eGalax serial touchscreen support

N: John Boyd
E: [email protected]
diff --git a/drivers/input/touchscreen/egalax_ts_serial.c b/drivers/input/touchscreen/egalax_ts_serial.c
index 375922d3a6d1..390b3a670bfa 100644
--- a/drivers/input/touchscreen/egalax_ts_serial.c
+++ b/drivers/input/touchscreen/egalax_ts_serial.c
@@ -2,7 +2,7 @@
/*
* EETI Egalax serial touchscreen driver
*
- * Copyright (c) 2015 Zoltán Böszörményi <[email protected]>
+ * Copyright (c) 2015 Zoltán Böszörményi <[email protected]>
*
* based on the
*
@@ -42,7 +42,7 @@ struct egalax {
struct serio *serio;
int idx;
u8 data[EGALAX_FORMAT_MAX_LENGTH];
- char phys[32];
+ char phys[40];
};

static void egalax_process_data(struct egalax *egalax)
@@ -185,6 +185,6 @@ static struct serio_driver egalax_drv = {
};
module_serio_driver(egalax_drv);

-MODULE_AUTHOR("Zoltán Böszörményi <[email protected]>");
+MODULE_AUTHOR("Zoltán Böszörményi <[email protected]>");
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE("GPL v2");
--
2.43.0