[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: and _PC_PIPE_BUF..
From: |
Robert Millan |
Subject: |
Re: and _PC_PIPE_BUF.. |
Date: |
Fri, 25 Apr 2003 12:20:54 +0200 |
User-agent: |
Mutt/1.5.4i |
tag 184345 - fixed
tag 184345 - patch
tag 184345 - pending
severity 184345 normal
retitle 184345 undefined PIPE_BUF bug wasn't fixed properly
thanks
as we've been already discussing in help-hurd mailing list, the
patch i sent had the problem that the situation when fpathconf
returns -1 was not considered. this could lead read() to take
a negative SIZE argument as unsigned, thus reading a quantity
near 2^32 bytes (or untill EOF). discussion follows...
On Thu, Apr 24, 2003 at 10:06:05AM +0200, Niels Möller wrote:
>
> > i'd better show an example, this is fixed code from gs. does it seem
> > correct to you? (considering the file descriptor is expected to be
> > a socket, if pipe_buf is -1 then something weird happened)
>
> Robert Millan <zeratul2@wanadoo.es> writes:
> > /* Get packet from the server. */
> > private int hpijs_get_pk(PK * pk, int fd)
> > {
> > #ifdef PIPE_BUF
> > return read(fd, pk, PIPE_BUF - 1);
> > #else
> > long int pipe_buf = fpathconf(fd, _PC_PIPE_BUF);
> > if (pipe_buf == -1)
> > /* no pipe_buf, so we fail */
> > return -1;
> > else
> > return read(fd, pk, pipe_buf - 1);
> > #endif
> > }
>
> To me, the code looks utterly broken. What is the allocated size for
> pk? If PIPE_BUF is larger than that size, and the other end of the
> pipe writes lots of data, you will get a buffer overflow, and
> depending on context, that can turn out to be a security hole big
> enough to drive a truck through it. The size argument to read must
> *never* be larger than the size of the buffer passed to read.
>
> Furthermore, I'm not sure what the code tries to use PIPE_BUF for. The
> only use of PIPE_BUF I have seen is when *writing* to a pipe, where
> you have a single process reading the pipe and multiple processes
> writing to it. Using PIPE_BUF when reading sounds like something that
> might make sense if you have a single writer and multiple readers on a
> pipe. Does gs do that? Such pipe usage seems pretty obscure to me.
>
> I would have expected some code like
>
> return read(fd, pk, sizeof(*pk))
>
> or, if it really depends on PIPE_BUF semantics,
>
> long int pipe_buf;
> #if PIPE_BUF
> pipe_buf = PIPE_BUF;
> #else
> pipe_buf = fpathconf(fd, _PC_PIPE_BUF);
> #endif
> if (pipe_buf < sizeof(*pk))
> /* pipe_buf too small (or not existing), so we fail */
> return -1;
> else
> return read(fd, pk, sizeof(*pk))
>
> And at last, the "-1" part of "PIPE_BUF - 1" also seems strange. The
> special PIPE_BUF semantics happen for request of size up to *or equal*
> to PIPE_BUF. Quoting the glibc manual:
>
> : Reading or writing pipe data is "atomic" if the size of data written
> : is not greater than `PIPE_BUF'. This means that the data transfer
> ^^^^^^^^^^^
> : seems to be an instantaneous unit, in that nothing else in the system
> : can observe a state in which it is partially complete. Atomic I/O may
> : not begin right away (it may need to wait for buffer space or for data),
> : but once it does begin it finishes immediately.
>
> Sorry if this isn't of much help, but I don't understand how that code
> is supposed to work.
>
> /Niels
>
--
Robert Millan
make: *** No rule to make target `war'. Stop.
Another world is possible - Just say no to genocide