Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20130511034947.GR20323@brightrain.aerifal.cx>
Date: Fri, 10 May 2013 23:49:47 -0400
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: procfs stdio writev problem

On Sun, May 05, 2013 at 08:49:50PM +0200, Jens wrote:
> >Looks to me at a quick glance like stdio needs something like (untested)
> >
> >--- ./src/stdio/__stdio_write.c~ 2012-12-01 22:56:34.156555480 +0000
> >+++ ./src/stdio/__stdio_write.c 2013-05-05 10:59:49.856504883 +0100
> >@@ -37,7 +37,7 @@
> > return iovcnt == 2 ? 0 : len-iov[0].iov_len;
> > }
> > rem -= cnt;
> >- if (cnt > iov[0].iov_len) {
> >+ if (cnt >= iov[0].iov_len) {
> > f->wpos = f->wbase = f->buf;
> > cnt -= iov[0].iov_len;
> > iov++; iovcnt--;
> >
> >In the case where the kernel exactly eats the iov you need to move
> >onto the next one rather than have a zero length write pointing just
> >after the existing one, as that could be an invalid address.
> 
> In this case its not the zero length that is the problem.
> The problem is that procfs treats each write (or apprently each part
> of the iov) as a separate operation.
> 
> So the first operation is "60" which is fine.
> The next one is "\n" which is invalid.
> So we get two operations instead of one.
> 
> The implementation in bash amounts to a printf("60") followed by
> putchar('\n');
> 
> The same thing in uclibc works as intended.
> 
> I guess I can patch bash, or use sysctl program.
> 
> AFAIK neither musl or procfs is doing anything wrong here, it just
> happens that a pure echo no longer works as it used to.

I think we can work around this (and possibly improve performance) by
modifying __overflow to store the new character into the buffer and
pass 0 length to __stdio_write. However, __stdio_write does not seem
prepared to deal with zero length specially, and although per the
standard it should not cause problems as long as the buffer is
non-empty, I don't trust Linux not to do stupid things with a zero iov
length on certain file types (like /proc).

So I think two changes are needed:

1. Modifying __overflow to submit the new char as part of the buffer.

2. Modifying __stdio_write to optimize the iov array, removing
   zero-length FILE-buffer or caller-buffer components. This will
   require some overhaul since the current code is ugly and using the
   iov index to make assumptions about whether it's operating on the
   FILE buffer or not.

It's this second change that I think should improve performance, since
Linux often behaves suboptimally on multiple iov components.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.