Re: [hackers] Re: [sbase][PATCH] dd: fix for ibs * count < obs

From: Michael Forney <mforney_AT_mforney.org>
Date: Wed, 12 Apr 2023 20:51:50 -0700

On 2022-11-29, phoebos <ben_AT_bvnf.space> wrote:
> Perhaps I should add some context.
>
> Running:
>
> yes | dd ibs=1 count=1
>
> reports that 512 bytes are read:
>
> 512+0 records in
> 1+0 records out
>
> Only one byte should be read, as occurs with GNU and busybox dd:
>
> 1+0 records in
> 0+1 records out
>
> I believe the reason for this bug in sbase is because of the comparison
> with obs in the below loop, which overlooks cases such as these.

Thanks for reporting this bug! Apologies for the delay.

> All feedback is welcome. Is this an appropriate fix to the bug?
>
> phoebos
>
> On Tue, Nov 22, 2022 at 16:28:35 +0000, phoebos wrote:
>> Previously, running `dd ibs=1 count=1` read 512 bytes rather than 1.
>> ---
>> dd.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/dd.c b/dd.c
>> index 6061048..4081eca 100644
>> --- a/dd.c
>> +++ b/dd.c
>> _AT_@ -174,7 +174,7 @@ main(int argc, char *argv[])
>> /* XXX: handle non-seekable files */
>> }
>> while (!eof && (count == -1 || ifull + ipart < count)) {
>> - while (ipos - opos < obs) {
>> + while (ipos - opos < obs && ifull + ipart < count) {

The count == -1 part of the outer condition is still important.
Otherwise, without a count argument, we never read any data.

Rather than duplicate this condition twice, I've pushed a slightly
different fix to just set eof once we've read the specified number of
blocks. I think this should fix the problem.

>> ret = read(ifd, buf + ipos, ibs);
>> if (ret == 0) {
>> eof = 1;
>> --
>> 2.38.1
Received on Thu Apr 13 2023 - 05:51:50 CEST