* [PATCH] thinkpad_acpi: fix: use scnprintf instead of snprintf.
@ 2021-01-06 6:36 YANG LI
2021-01-06 9:01 ` Joe Perches
0 siblings, 1 reply; 4+ messages in thread
From: YANG LI @ 2021-01-06 6:36 UTC (permalink / raw)
To: hdegoede
Cc: mgross, ibm-acpi, ibm-acpi-devel, platform-driver-x86,
linux-kernel, YANG LI
The snprintf() function returns the number of characters which would
have been printed if there were enough space, but the scnprintf()
returns the number of characters which were actually printed. If the
buffer is not large enough, then using snprintf() would result in a
read overflow and an information leak. This error was found with the
help of coccicheck.
Signed-off-by: YANG LI <abaci-bugfix@linux.alibaba.com>
Reported-by: Abaci <abaci@linux.alibaba.com>
---
drivers/platform/x86/thinkpad_acpi.c | 62 ++++++++++++++++++------------------
1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index e03df28..c29a639 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -1340,7 +1340,7 @@ static ssize_t tpacpi_rfk_sysfs_enable_show(const enum tpacpi_rfk_id id,
return status;
}
- return snprintf(buf, PAGE_SIZE, "%d\n",
+ return scnprintf(buf, PAGE_SIZE, "%d\n",
(status == TPACPI_RFK_RADIO_ON) ? 1 : 0);
}
@@ -1433,14 +1433,14 @@ static int tpacpi_rfk_procfs_write(const enum tpacpi_rfk_id id, char *buf)
/* interface_version --------------------------------------------------- */
static ssize_t interface_version_show(struct device_driver *drv, char *buf)
{
- return snprintf(buf, PAGE_SIZE, "0x%08x\n", TPACPI_SYSFS_VERSION);
+ return scnprintf(buf, PAGE_SIZE, "0x%08x\n", TPACPI_SYSFS_VERSION);
}
static DRIVER_ATTR_RO(interface_version);
/* debug_level --------------------------------------------------------- */
static ssize_t debug_level_show(struct device_driver *drv, char *buf)
{
- return snprintf(buf, PAGE_SIZE, "0x%04x\n", dbg_level);
+ return scnprintf(buf, PAGE_SIZE, "0x%04x\n", dbg_level);
}
static ssize_t debug_level_store(struct device_driver *drv, const char *buf,
@@ -1460,7 +1460,7 @@ static ssize_t debug_level_store(struct device_driver *drv, const char *buf,
/* version ------------------------------------------------------------- */
static ssize_t version_show(struct device_driver *drv, char *buf)
{
- return snprintf(buf, PAGE_SIZE, "%s v%s\n",
+ return scnprintf(buf, PAGE_SIZE, "%s v%s\n",
TPACPI_DESC, TPACPI_VERSION);
}
static DRIVER_ATTR_RO(version);
@@ -1472,7 +1472,7 @@ static ssize_t version_show(struct device_driver *drv, char *buf)
/* wlsw_emulstate ------------------------------------------------------ */
static ssize_t wlsw_emulstate_show(struct device_driver *drv, char *buf)
{
- return snprintf(buf, PAGE_SIZE, "%d\n", !!tpacpi_wlsw_emulstate);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", !!tpacpi_wlsw_emulstate);
}
static ssize_t wlsw_emulstate_store(struct device_driver *drv, const char *buf,
@@ -1495,7 +1495,7 @@ static ssize_t wlsw_emulstate_store(struct device_driver *drv, const char *buf,
/* bluetooth_emulstate ------------------------------------------------- */
static ssize_t bluetooth_emulstate_show(struct device_driver *drv, char *buf)
{
- return snprintf(buf, PAGE_SIZE, "%d\n", !!tpacpi_bluetooth_emulstate);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", !!tpacpi_bluetooth_emulstate);
}
static ssize_t bluetooth_emulstate_store(struct device_driver *drv,
@@ -1515,7 +1515,7 @@ static ssize_t bluetooth_emulstate_store(struct device_driver *drv,
/* wwan_emulstate ------------------------------------------------- */
static ssize_t wwan_emulstate_show(struct device_driver *drv, char *buf)
{
- return snprintf(buf, PAGE_SIZE, "%d\n", !!tpacpi_wwan_emulstate);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", !!tpacpi_wwan_emulstate);
}
static ssize_t wwan_emulstate_store(struct device_driver *drv, const char *buf,
@@ -1535,7 +1535,7 @@ static ssize_t wwan_emulstate_store(struct device_driver *drv, const char *buf,
/* uwb_emulstate ------------------------------------------------- */
static ssize_t uwb_emulstate_show(struct device_driver *drv, char *buf)
{
- return snprintf(buf, PAGE_SIZE, "%d\n", !!tpacpi_uwb_emulstate);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", !!tpacpi_uwb_emulstate);
}
static ssize_t uwb_emulstate_store(struct device_driver *drv, const char *buf,
@@ -2745,7 +2745,7 @@ static ssize_t hotkey_enable_show(struct device *dev,
if (res)
return res;
- return snprintf(buf, PAGE_SIZE, "%d\n", status);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", status);
}
static ssize_t hotkey_enable_store(struct device *dev,
@@ -2773,7 +2773,7 @@ static ssize_t hotkey_mask_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return snprintf(buf, PAGE_SIZE, "0x%08x\n", hotkey_user_mask);
+ return scnprintf(buf, PAGE_SIZE, "0x%08x\n", hotkey_user_mask);
}
static ssize_t hotkey_mask_store(struct device *dev,
@@ -2821,7 +2821,7 @@ static ssize_t hotkey_bios_mask_show(struct device *dev,
{
printk_deprecated_attribute("hotkey_bios_mask",
"This attribute is useless.");
- return snprintf(buf, PAGE_SIZE, "0x%08x\n", hotkey_orig_mask);
+ return scnprintf(buf, PAGE_SIZE, "0x%08x\n", hotkey_orig_mask);
}
static DEVICE_ATTR_RO(hotkey_bios_mask);
@@ -2831,7 +2831,7 @@ static ssize_t hotkey_all_mask_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return snprintf(buf, PAGE_SIZE, "0x%08x\n",
+ return scnprintf(buf, PAGE_SIZE, "0x%08x\n",
hotkey_all_mask | hotkey_source_mask);
}
@@ -2842,7 +2842,7 @@ static ssize_t hotkey_adaptive_all_mask_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return snprintf(buf, PAGE_SIZE, "0x%08x\n",
+ return scnprintf(buf, PAGE_SIZE, "0x%08x\n",
hotkey_adaptive_all_mask | hotkey_source_mask);
}
@@ -2853,7 +2853,7 @@ static ssize_t hotkey_recommended_mask_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return snprintf(buf, PAGE_SIZE, "0x%08x\n",
+ return scnprintf(buf, PAGE_SIZE, "0x%08x\n",
(hotkey_all_mask | hotkey_source_mask)
& ~hotkey_reserved_mask);
}
@@ -2867,7 +2867,7 @@ static ssize_t hotkey_source_mask_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return snprintf(buf, PAGE_SIZE, "0x%08x\n", hotkey_source_mask);
+ return scnprintf(buf, PAGE_SIZE, "0x%08x\n", hotkey_source_mask);
}
static ssize_t hotkey_source_mask_store(struct device *dev,
@@ -2918,7 +2918,7 @@ static ssize_t hotkey_poll_freq_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return snprintf(buf, PAGE_SIZE, "%d\n", hotkey_poll_freq);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", hotkey_poll_freq);
}
static ssize_t hotkey_poll_freq_store(struct device *dev,
@@ -2960,7 +2960,7 @@ static ssize_t hotkey_radio_sw_show(struct device *dev,
/* Opportunistic update */
tpacpi_rfk_update_hwblock_state((res == TPACPI_RFK_RADIO_OFF));
- return snprintf(buf, PAGE_SIZE, "%d\n",
+ return scnprintf(buf, PAGE_SIZE, "%d\n",
(res == TPACPI_RFK_RADIO_OFF) ? 0 : 1);
}
@@ -2983,7 +2983,7 @@ static ssize_t hotkey_tablet_mode_show(struct device *dev,
if (res < 0)
return res;
- return snprintf(buf, PAGE_SIZE, "%d\n", !!s);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", !!s);
}
static DEVICE_ATTR_RO(hotkey_tablet_mode);
@@ -3000,7 +3000,7 @@ static ssize_t hotkey_wakeup_reason_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return snprintf(buf, PAGE_SIZE, "%d\n", hotkey_wakeup_reason);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", hotkey_wakeup_reason);
}
static DEVICE_ATTR(wakeup_reason, S_IRUGO, hotkey_wakeup_reason_show, NULL);
@@ -3016,7 +3016,7 @@ static ssize_t hotkey_wakeup_hotunplug_complete_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return snprintf(buf, PAGE_SIZE, "%d\n", hotkey_autosleep_ack);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", hotkey_autosleep_ack);
}
static DEVICE_ATTR(wakeup_hotunplug_complete, S_IRUGO,
@@ -3051,7 +3051,7 @@ static ssize_t adaptive_kbd_mode_show(struct device *dev,
if (current_mode < 0)
return current_mode;
- return snprintf(buf, PAGE_SIZE, "%d\n", current_mode);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", current_mode);
}
static ssize_t adaptive_kbd_mode_store(struct device *dev,
@@ -6295,7 +6295,7 @@ static int thermal_get_sensor(int idx, s32 *value)
case TPACPI_THERMAL_ACPI_UPDT:
if (idx <= 7) {
- snprintf(tmpi, sizeof(tmpi), "TMP%c", '0' + idx);
+ scnprintf(tmpi, sizeof(tmpi), "TMP%c", '0' + idx);
if (!acpi_evalf(ec_handle, NULL, "UPDT", "v"))
return -EIO;
if (!acpi_evalf(ec_handle, &t, tmpi, "d"))
@@ -6307,7 +6307,7 @@ static int thermal_get_sensor(int idx, s32 *value)
case TPACPI_THERMAL_ACPI_TMP07:
if (idx <= 7) {
- snprintf(tmpi, sizeof(tmpi), "TMP%c", '0' + idx);
+ scnprintf(tmpi, sizeof(tmpi), "TMP%c", '0' + idx);
if (!acpi_evalf(ec_handle, &t, tmpi, "d"))
return -EIO;
if (t > 127 || t < -127)
@@ -6387,7 +6387,7 @@ static ssize_t thermal_temp_input_show(struct device *dev,
if (value == TPACPI_THERMAL_SENSOR_NA)
return -ENXIO;
- return snprintf(buf, PAGE_SIZE, "%d\n", value);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", value);
}
#define THERMAL_SENSOR_ATTR_TEMP(_idxA, _idxB) \
@@ -7618,10 +7618,10 @@ static int __init volume_create_alsa_mixer(void)
sizeof(card->driver));
strlcpy(card->shortname, TPACPI_ALSA_SHRTNAME,
sizeof(card->shortname));
- snprintf(card->mixername, sizeof(card->mixername), "ThinkPad EC %s",
+ scnprintf(card->mixername, sizeof(card->mixername), "ThinkPad EC %s",
(thinkpad_id.ec_version_str) ?
thinkpad_id.ec_version_str : "(unknown)");
- snprintf(card->longname, sizeof(card->longname),
+ scnprintf(card->longname, sizeof(card->longname),
"%s at EC reg 0x%02x, fw %s", card->shortname, TP_EC_AUDIO,
(thinkpad_id.ec_version_str) ?
thinkpad_id.ec_version_str : "unknown");
@@ -8588,7 +8588,7 @@ static ssize_t fan_pwm1_enable_show(struct device *dev,
} else
mode = 1;
- return snprintf(buf, PAGE_SIZE, "%d\n", mode);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", mode);
}
static ssize_t fan_pwm1_enable_store(struct device *dev,
@@ -8654,7 +8654,7 @@ static ssize_t fan_pwm1_show(struct device *dev,
if (status > 7)
status = 7;
- return snprintf(buf, PAGE_SIZE, "%u\n", (status * 255) / 7);
+ return scnprintf(buf, PAGE_SIZE, "%u\n", (status * 255) / 7);
}
static ssize_t fan_pwm1_store(struct device *dev,
@@ -8707,7 +8707,7 @@ static ssize_t fan_fan1_input_show(struct device *dev,
if (res < 0)
return res;
- return snprintf(buf, PAGE_SIZE, "%u\n", speed);
+ return scnprintf(buf, PAGE_SIZE, "%u\n", speed);
}
static DEVICE_ATTR(fan1_input, S_IRUGO, fan_fan1_input_show, NULL);
@@ -8724,7 +8724,7 @@ static ssize_t fan_fan2_input_show(struct device *dev,
if (res < 0)
return res;
- return snprintf(buf, PAGE_SIZE, "%u\n", speed);
+ return scnprintf(buf, PAGE_SIZE, "%u\n", speed);
}
static DEVICE_ATTR(fan2_input, S_IRUGO, fan_fan2_input_show, NULL);
@@ -8732,7 +8732,7 @@ static ssize_t fan_fan2_input_show(struct device *dev,
/* sysfs fan fan_watchdog (hwmon driver) ------------------------------- */
static ssize_t fan_watchdog_show(struct device_driver *drv, char *buf)
{
- return snprintf(buf, PAGE_SIZE, "%u\n", fan_watchdog_maxinterval);
+ return scnprintf(buf, PAGE_SIZE, "%u\n", fan_watchdog_maxinterval);
}
static ssize_t fan_watchdog_store(struct device_driver *drv, const char *buf,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] thinkpad_acpi: fix: use scnprintf instead of snprintf.
2021-01-06 6:36 [PATCH] thinkpad_acpi: fix: use scnprintf instead of snprintf YANG LI
@ 2021-01-06 9:01 ` Joe Perches
2021-01-06 9:14 ` Hans de Goede
2021-01-15 19:27 ` Henrique de Moraes Holschuh
0 siblings, 2 replies; 4+ messages in thread
From: Joe Perches @ 2021-01-06 9:01 UTC (permalink / raw)
To: YANG LI, hdegoede
Cc: mgross, ibm-acpi, ibm-acpi-devel, platform-driver-x86, linux-kernel
On Wed, 2021-01-06 at 14:36 +0800, YANG LI wrote:
> The snprintf() function returns the number of characters which would
> have been printed if there were enough space, but the scnprintf()
> returns the number of characters which were actually printed. If the
> buffer is not large enough, then using snprintf() would result in a
> read overflow and an information leak. This error was found with the
> help of coccicheck.
In all cases, the buffer _is_ large enough.
tmpi is length 5 and ok.
include/sound/core.h: char shortname[32]; /* short name of this soundcard */
include/sound/core.h: char longname[80]; /* name of this soundcard */
include/sound/core.h: char mixername[80]; /* mixer name */
_show function lengths are OK for all the uses with PAGE_SIZE.
And it's probably better to use sysfs_emit for all the _show functions
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] thinkpad_acpi: fix: use scnprintf instead of snprintf.
2021-01-06 9:01 ` Joe Perches
@ 2021-01-06 9:14 ` Hans de Goede
2021-01-15 19:27 ` Henrique de Moraes Holschuh
1 sibling, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2021-01-06 9:14 UTC (permalink / raw)
To: Joe Perches, YANG LI
Cc: mgross, ibm-acpi, ibm-acpi-devel, platform-driver-x86, linux-kernel
Hi,
On 1/6/21 10:01 AM, Joe Perches wrote:
> On Wed, 2021-01-06 at 14:36 +0800, YANG LI wrote:
>> The snprintf() function returns the number of characters which would
>> have been printed if there were enough space, but the scnprintf()
>> returns the number of characters which were actually printed. If the
>> buffer is not large enough, then using snprintf() would result in a
>> read overflow and an information leak. This error was found with the
>> help of coccicheck.
>
> In all cases, the buffer _is_ large enough.
>
> tmpi is length 5 and ok.
> include/sound/core.h: char shortname[32]; /* short name of this soundcard */
> include/sound/core.h: char longname[80]; /* name of this soundcard */
> include/sound/core.h: char mixername[80]; /* mixer name */
>
> _show function lengths are OK for all the uses with PAGE_SIZE.
> And it's probably better to use sysfs_emit for all the _show functions
Yes, please send a v2 with the following changes:
1. Use sysfs_emit in all the sysfs read functions
2. Do not replace snprintf with scnprintf when the return value is not used, that is just needless churn
3. Update the commit message to reflect 1.
Regards,
Hans
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] thinkpad_acpi: fix: use scnprintf instead of snprintf.
2021-01-06 9:01 ` Joe Perches
2021-01-06 9:14 ` Hans de Goede
@ 2021-01-15 19:27 ` Henrique de Moraes Holschuh
1 sibling, 0 replies; 4+ messages in thread
From: Henrique de Moraes Holschuh @ 2021-01-15 19:27 UTC (permalink / raw)
To: Joe Perches
Cc: YANG LI, hdegoede, mgross, ibm-acpi, ibm-acpi-devel,
platform-driver-x86, linux-kernel
On Wed, 06 Jan 2021, Joe Perches wrote:
> On Wed, 2021-01-06 at 14:36 +0800, YANG LI wrote:
> > The snprintf() function returns the number of characters which would
> > have been printed if there were enough space, but the scnprintf()
> > returns the number of characters which were actually printed. If the
> > buffer is not large enough, then using snprintf() would result in a
> > read overflow and an information leak. This error was found with the
> > help of coccicheck.
>
> In all cases, the buffer _is_ large enough.
Thank you for double-checking!
> _show function lengths are OK for all the uses with PAGE_SIZE.
> And it's probably better to use sysfs_emit for all the _show functions
Indeed.
--
Henrique Holschuh
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-15 19:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-06 6:36 [PATCH] thinkpad_acpi: fix: use scnprintf instead of snprintf YANG LI
2021-01-06 9:01 ` Joe Perches
2021-01-06 9:14 ` Hans de Goede
2021-01-15 19:27 ` Henrique de Moraes Holschuh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).