[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Fri, 16 Sep 2011 06:16:07 +0400
From: Solar Designer <solar@...nwall.com>
To: owl-dev@...ts.openwall.com
Subject: cpio write(2) return value checks
Vasiliy, Dmitry -
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.
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? 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.
Thanks,
Alexander
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.
