[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPLrYEQoOwk4AaUGffhnn4cr0QgfEO2rQBz1z6jGbJyKYKUQhw@mail.gmail.com>
Date: Fri, 16 Sep 2011 18:24:16 +0200
From: Daniel Cegiełka <daniel.cegielka@...il.com>
To: owl-dev@...ts.openwall.com
Subject: Re: cpio write(2) return value checks
Can you check cpio-2.10.90-owl-fixes.diff?
I think there is a conflict with cpio-2.9-alt-open-O_EXCL.diff.
regards,
daniel
2011/9/16 Vasiliy Kulikov <segoon@...nwall.com>:
> Solar, Dmitry,
>
> On Fri, Sep 16, 2011 at 06:16 +0400, Solar Designer wrote:
>> On Thu, Sep 15, 2011 at 09:15:55PM +0400, Owl CVS (segoon) wrote:
>> > Modified Files:
>> > cpio-2.10.90-owl-fixes.diff cpio.spec
>> > Added Files:
>> > cpio-2.10.90-owl-warnings.diff
>> > Log Message:
>> > Added checks of write(2) and lseek(2) return codes to -owl-fixes patch.
>> [...]
>> > +@@ -1176,7 +1177,8 @@ sparse_write (int fildes, char *buf, uns
>> > + case not_in_zeros :
>> > + if (buf_all_zeros (buf, DISKBLOCKSIZE))
>> > + {
>> > +- write_rc = write (fildes, cur_write_start, write_count);
>> > ++ if (write (fildes, cur_write_start, write_count) != write_count)
>> > ++ return -1;
>> > + seek_count = DISKBLOCKSIZE;
>> > + state = in_zeros;
>> > + }
>> > +@@ -1197,7 +1199,8 @@ sparse_write (int fildes, char *buf, uns
>> > + break;
>> > +
>> > + case not_in_zeros :
>> > +- write_rc = write (fildes, cur_write_start, write_count);
>> > ++ if (write (fildes, cur_write_start, write_count) != write_count)
>> > ++ return -1;
>> > + delayed_seek_count = 0;
>> > + break;
>> > + }
>> [...]
>> > +@@ -1206,10 +1209,12 @@ sparse_write (int fildes, char *buf, uns
>> [...]
>> > +- write_rc = write (fildes, buf, leftover_bytes_count);
>> > ++ if (write (fildes, buf, leftover_bytes_count) != leftover_bytes_count)
>> > ++ return -1;
>> > + }
>> > + return nbyte;
>> > + }
>>
>> Wouldn't it be better for the code to handle partial writes instead, and
>> only treat 0 and -1 returns from write(2) as errors?
>>
>> Was the write_rc variable unused? If so, perhaps that was the bug.
>
> Yes, I've removed it in cpio-2.10.90-owl-warnings.diff.
>
>> Maybe we need to add a write_loop() function (like we have in some other
>> packages) and use it in these places?
>>
>> Are these issues possibly already dealt with in a newer version of cpio,
>> though?
>
> No. In 2.11 there is no check and write_rc is still unused.
>
>> If so, we'd want not to waste time on our own fix (simple
>> checks for "!=" and "return -1" like the above would be fine for now).
>>
>> Also, doesn't the caller to sparse_write() expect a valid errno on a -1
>> return value? (I have no idea, I didn't check.) write(2) only sets
>> errno when it returns -1; on a partial write, it does not.
>
> Yes, good point.
>
>
> cpio-2.11 is a bugfix release:
>
> https://www.gnu.org/s/cpio/
>
> Fix mt build.
> In copy-in mode, if directory attributes do not permit writing to
> it, setting them is delayed until the end of run. This allows to
> correctly extract files in such directories.
> In copy-in mode, permissions of a directory are restored if it
> appears in the file list after files in it (e.g. in listings produced by
> find . -depth). This fixes debian bug #458079.
> Fix possible memory overflow in the rmt client code (CVE-2010-0624).
>
> (The latter we have fixed in cpio-2.10.90-up-bound.diff).
>
>
> Probably we should report these to upstream, wait for 2.12 with the
> fixes and upgrade to 2.12?
>
>
> Thanks,
>
> --
> Vasiliy
>
Download attachment "cpio-2.10.90-owl-fixes.diff" of type "application/octet-stream" (7604 bytes)
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.
