linux-kernel - [PATCH 2/2] drivers/rtc/rtc-ds1685.c: don't try to micromanage sysfs output size

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]

Date:	Tue, 24 Nov 2015 14:51:24 +0100
From:	Rasmus Villemoes <linux@...musvillemoes.dk>
To:	Joshua Kinard <kumba@...too.org>,
	Alessandro Zummo <a.zummo@...ertech.it>,
	Alexandre Belloni <alexandre.belloni@...e-electrons.com>
Cc:	Rasmus Villemoes <linux@...musvillemoes.dk>,
	rtc-linux@...glegroups.com, linux-kernel@...r.kernel.org
Subject: [PATCH 2/2] drivers/rtc/rtc-ds1685.c: don't try to micromanage sysfs output size

...and don't do it wrong.

"not ok or N/A" has length 13. Add the trailing newline, and the
snprintf return value will be 14. However, we lied to snprintf and
told it that only 13 bytes were available. Hence snprintf has only
written "not ok or N/" and a trailing '\0' to the buffer. Next we
continue lying, this time to the upper sysfs layer, claiming that we
wrote 14 meaningful bytes to the buffer. That'll make the upper layer
copy "not ok or N/" plus two nul bytes to user space (one nul byte
from snprintf, the other since sysfs takes care to clear the buffer
before giving it to the ->show method).

In the other cases, the claimed buffer size is closer to sufficient,
but we'll still get a nul byte instead of a newline written to user
space.  There's absolutely no reason to try to predict the output
size, and there's plenty of room in the buffer, so just use sprintf.

Signed-off-by: Rasmus Villemoes <linux@...musvillemoes.dk>
---
 drivers/rtc/rtc-ds1685.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-ds1685.c b/drivers/rtc/rtc-ds1685.c
index 5038122aa8de..535050fc5e9f 100644
--- a/drivers/rtc/rtc-ds1685.c
+++ b/drivers/rtc/rtc-ds1685.c
@@ -1114,7 +1114,7 @@ ds1685_rtc_sysfs_battery_show(struct device *dev,
 
 	ctrld = rtc->read(rtc, RTC_CTRL_D);
 
-	return snprintf(buf, 13, "%s\n",
+	return sprintf(buf, "%s\n",
 			(ctrld & RTC_CTRL_D_VRT) ? "ok" : "not ok or N/A");
 }
 static DEVICE_ATTR(battery, S_IRUGO, ds1685_rtc_sysfs_battery_show, NULL);
@@ -1137,7 +1137,7 @@ ds1685_rtc_sysfs_auxbatt_show(struct device *dev,
 	ctrl4a = rtc->read(rtc, RTC_EXT_CTRL_4A);
 	ds1685_rtc_switch_to_bank0(rtc);
 
-	return snprintf(buf, 13, "%s\n",
+	return sprintf(buf, "%s\n",
 			(ctrl4a & RTC_CTRL_4A_VRT2) ? "ok" : "not ok or N/A");
 }
 static DEVICE_ATTR(auxbatt, S_IRUGO, ds1685_rtc_sysfs_auxbatt_show, NULL);
@@ -1160,9 +1160,7 @@ ds1685_rtc_sysfs_serial_show(struct device *dev,
 	ds1685_rtc_get_ssn(rtc, ssn);
 	ds1685_rtc_switch_to_bank0(rtc);
 
-	return snprintf(buf, 24, "%8phC\n", ssn);
-
-	return 0;
+	return sprintf(buf, "%8phC\n", ssn);
 }
 static DEVICE_ATTR(serial, S_IRUGO, ds1685_rtc_sysfs_serial_show, NULL);
 
@@ -1285,7 +1283,7 @@ ds1685_rtc_sysfs_ctrl_regs_show(struct device *dev,
 	tmp = rtc->read(rtc, reg_info->reg) & reg_info->bit;
 	ds1685_rtc_switch_to_bank0(rtc);
 
-	return snprintf(buf, 2, "%d\n", (tmp ? 1 : 0));
+	return sprintf(buf, "%d\n", (tmp ? 1 : 0));
 }
 
 /**
@@ -1621,7 +1619,7 @@ ds1685_rtc_sysfs_time_regs_show(struct device *dev,
 	tmp = ds1685_rtc_bcd2bin(rtc, tmp, bcd_reg_info->mask,
 				 bin_reg_info->mask);
 
-	return snprintf(buf, 4, "%d\n", tmp);
+	return sprintf(buf, "%d\n", tmp);
 }
 
 /**
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/