#547902 dash: 'read' builtin reads only the first character from pipes or /proc files

Package:
dash
Source:
dash
Description:
POSIX-compliant shell
Submitter:
Corin
Date:
2024-05-19 10:36:04 UTC
Severity:
important
Tags:
#547902#5
Date:
2009-09-22 13:49:40 UTC
From:
To:
The 'read' command seems to only read the first character and not the whole line.

Example / comparison with bash:

# /bin/dash
# read MAX </proc/sys/net/netfilter/nf_conntrack_max
# echo $MAX
2

# /bin/bash
# read MAX </proc/sys/net/netfilter/nf_conntrack_max
# echo $MAX
262144
#547902#12
Date:
2009-09-29 11:59:55 UTC
From:
To:
Hi, this doesn't look like a bug in dash, but in the kernel's
implementation of the /proc/sys/net/ hierarchy.  If you copy the file to
/tmp/, it works just fine.

cat <<EOT >t.c
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

int main(int argc, char **argv) {
	char c;
	int fd = open(argv[1], O_RDONLY);
	if (fd == -1) return 1;
	for (;;) {
		int i = read(fd, &c, 1);
		if (i == 0) break;
		if (i == -1) return 2;
		if (write(1, &c, 1) == -1) return 3;
	}
	write(1, "\n", 1);
	return 0;
}
EOT
gcc t.c
./a.out /proc/sys/net/unix/max_dgram_qlen
cat /proc/sys/net/unix/max_dgram_qlen
cp /proc/sys/net/unix/max_dgram_qlen /tmp/
./a.out /tmp/max_dgram_qlen

Regards, Gerrit.
#547902#17
Date:
2009-10-08 11:34:40 UTC
From:
To:
Hi,

I'm still not sure if it's a kernel bug/misbehavior, as bash works fine
on the same system. And even if it's a kernel bug, shouldn't dash
contain the same work-around as bash seems to have?

Fact is, when upgrading debian (and so dash becomes the default shell)
some applications break because of this. One example are munin plugins,
which is where I discovered the bug/ regression. Changing the shebang
line in the munin plugins from /bin/sh (symlink to /bin/dash) to
/bin/bash solves the problems.

Corin
#547902#26
Date:
2009-10-13 11:13:01 UTC
From:
To:
tags 547902 + patch
quit

Hi, please see http://bugs.debian.org/547902

Here's a patch from Christian Hohnstaedt <chohnstaedt@innominate.com>

Thanks, Gerrit.

---
Testparameter:
for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19; do dd if=/proc/sys/net/ipv4/tcp_wmem bs=$i 2>/dev/null; done

for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19; do dd if=/proc/sys/net/ipv4/tcp_wmem skip=$i bs=1 2>/dev/null |wc -c ; done

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 9270125..038df14 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2117,17 +2117,16 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
 #define TMPBUFLEN 21
 	int *i, vleft, first=1, neg, val;
 	unsigned long lval;
-	size_t left, len;
+	size_t left, len, off;

 	char buf[TMPBUFLEN], *p;
 	char __user *s = buffer;

-	if (!tbl_data || !table->maxlen || !*lenp ||
-	    (*ppos && !write)) {
+	if (!tbl_data || !table->maxlen || !*lenp) {
 		*lenp = 0;
 		return 0;
 	}
-
+	off = 0;
 	i = (int *) tbl_data;
 	vleft = table->maxlen / sizeof(*i);
 	left = *lenp;
@@ -2176,25 +2175,31 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
 			if (conv(&neg, &lval, i, 1, data))
 				break;
 		} else {
+			loff_t diff;
 			p = buf;
-			if (!first)
-				*p++ = '\t';
-
 			if (conv(&neg, &lval, i, 0, data))
 				break;

-			sprintf(p, "%s%lu", neg ? "-" : "", lval);
-			len = strlen(buf);
-			if (len > left)
-				len = left;
-			if(copy_to_user(s, buf, len))
-				return -EFAULT;
-			left -= len;
-			s += len;
+			len = sprintf(p, "%s%s%lu", first ? "" : "\t",
+						neg ? "-" : "", lval);
+			diff = *ppos - off;
+			off += len;
+			if (diff > 0 && diff < len) {
+				p += diff;
+				len -= diff;
+			}
+			if (off > *ppos) {
+				if (len > left)
+					len = left;
+				if(copy_to_user(s, p, len))
+					return -EFAULT;
+				left -= len;
+				s += len;
+			}
 		}
 	}

-	if (!write && !first && left) {
+	if (!write && !first && left && off >= *ppos) {
 		if(put_user('\n', s))
 			return -EFAULT;
 		left--, s++;
#547902#31
Date:
2009-10-13 11:13:01 UTC
From:
To:
tags 547902 + patch
quit

Hi, please see http://bugs.debian.org/547902

Here's a patch from Christian Hohnstaedt <chohnstaedt@innominate.com>

Thanks, Gerrit.

---
Testparameter:
for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19; do dd if=/proc/sys/net/ipv4/tcp_wmem bs=$i 2>/dev/null; done

for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19; do dd if=/proc/sys/net/ipv4/tcp_wmem skip=$i bs=1 2>/dev/null |wc -c ; done

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 9270125..038df14 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2117,17 +2117,16 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
 #define TMPBUFLEN 21
 	int *i, vleft, first=1, neg, val;
 	unsigned long lval;
-	size_t left, len;
+	size_t left, len, off;

 	char buf[TMPBUFLEN], *p;
 	char __user *s = buffer;

-	if (!tbl_data || !table->maxlen || !*lenp ||
-	    (*ppos && !write)) {
+	if (!tbl_data || !table->maxlen || !*lenp) {
 		*lenp = 0;
 		return 0;
 	}
-
+	off = 0;
 	i = (int *) tbl_data;
 	vleft = table->maxlen / sizeof(*i);
 	left = *lenp;
@@ -2176,25 +2175,31 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
 			if (conv(&neg, &lval, i, 1, data))
 				break;
 		} else {
+			loff_t diff;
 			p = buf;
-			if (!first)
-				*p++ = '\t';
-
 			if (conv(&neg, &lval, i, 0, data))
 				break;

-			sprintf(p, "%s%lu", neg ? "-" : "", lval);
-			len = strlen(buf);
-			if (len > left)
-				len = left;
-			if(copy_to_user(s, buf, len))
-				return -EFAULT;
-			left -= len;
-			s += len;
+			len = sprintf(p, "%s%s%lu", first ? "" : "\t",
+						neg ? "-" : "", lval);
+			diff = *ppos - off;
+			off += len;
+			if (diff > 0 && diff < len) {
+				p += diff;
+				len -= diff;
+			}
+			if (off > *ppos) {
+				if (len > left)
+					len = left;
+				if(copy_to_user(s, p, len))
+					return -EFAULT;
+				left -= len;
+				s += len;
+			}
 		}
 	}

-	if (!write && !first && left) {
+	if (!write && !first && left && off >= *ppos) {
 		if(put_user('\n', s))
 			return -EFAULT;
 		left--, s++;
#547902#36
Date:
2009-10-17 00:36:03 UTC
From:
To:
[...]

Let us know when this change is accepted upstream.

Ben.
#547902#43
Date:
2009-11-13 11:00:26 UTC
From:
To:
Sorry, I neither forwarded it upstream, nor follow upstream development.

Regards, Gerrit.
#547902#48
Date:
2009-11-13 12:34:10 UTC
From:
To:
Then we will not apply your patch.

Ben.
#547902#53
Date:
2009-11-13 14:29:20 UTC
From:
To:
FYI.  Sorry, I can't help further.

Regards, Gerrit.

----- Forwarded message from Ben Hutchings <ben@decadent.org.uk> -----

From: Ben Hutchings <ben@decadent.org.uk>
To: Gerrit Pape <pape@dbnbgs.smarden.org>, 547902@bugs.debian.org
Date: Fri, 13 Nov 2009 12:34:10 +0000
Subject: Re: Bug#547902: dash only reads the first character from pipe
 (bash reads whole line)

Then we will not apply your patch.

Ben.



----- End forwarded message -----
#547902#58
Date:
2009-11-18 21:13:48 UTC
From:
To:
[ Adding Christian to CC: ]

Christian, did you submit your patch upstream?

Cheers,
        Moritz
#547902#61
Date:
2009-11-18 21:13:48 UTC
From:
To:
[ Adding Christian to CC: ]

Christian, did you submit your patch upstream?

Cheers,
        Moritz
#547902#66
Date:
2009-11-19 08:27:47 UTC
From:
To:
No, not yet.

It still has the problem that reading the value(s) bytewise returns garbage
if the value(s) change during reading.

But I can try to send it to lkml and see what they say.
Shall I point to this bug-report and may I add a "Tested-by" line ?

	Christian
Christian Hohnstaedt
#547902#69
Date:
2009-11-19 08:27:47 UTC
From:
To:
No, not yet.

It still has the problem that reading the value(s) bytewise returns garbage
if the value(s) change during reading.

But I can try to send it to lkml and see what they say.
Shall I point to this bug-report and may I add a "Tested-by" line ?

	Christian
Christian Hohnstaedt
#547902#74
Date:
2009-11-24 21:16:06 UTC
From:
To:
Please keep this bug CCed if you submit it.

I didn't test it myself, but maybe Corin can give feedback on this.

Cheers,
        Moritz
#547902#77
Date:
2009-11-24 21:16:06 UTC
From:
To:
Please keep this bug CCed if you submit it.

I didn't test it myself, but maybe Corin can give feedback on this.

Cheers,
        Moritz
#547902#82
Date:
2011-05-06 19:15:45 UTC
From:
To:
Closing due to lack of activity.

Cheers,
        Moritz
#547902#87
Date:
2011-05-06 19:15:45 UTC
From:
To:
Closing due to lack of activity.

Cheers,
        Moritz
#547902#92
Date:
2011-05-06 19:26:07 UTC
From:
To:
reassign 547906 dash 0.5.6.1-1~exp2
reopen 547906
tags 547906 = patch
quit

Moritz Muehlenhoff wrote:

I'll take it.  Will reassign back to linux-2.6 if there is progress.
#547902#107
Date:
2015-11-14 12:53:04 UTC
From:
To:
This problem is known upstream, see
<http://thread.gmane.org/gmane.comp.shells.dash/1174>.

The suggested workaround is

     MAX=$(cat /proc/.../file)

Regards,