linux-kernel - Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 9 Feb 2011 11:07:19 +0100
From:	Belisko Marek <marek.belisko@...il.com>
To:	Dan Carpenter <error27@...il.com>,
	Belisko Marek <marek.belisko@...il.com>,
	Marek Belisko <marek.belisko@...n-nandra.com>, gregkh@...e.de,
	devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.

On Tue, Feb 8, 2011 at 5:35 PM, Dan Carpenter <error27@...il.com> wrote:
> On Tue, Feb 08, 2011 at 02:40:49PM +0100, Belisko Marek wrote:
>> On Wed, Jan 26, 2011 at 3:30 PM, Dan Carpenter <error27@...il.com> wrote:
>> > Also when it does:
>> >        memcpy(ft1000dev->tx_buf, *pUcFile, byte_length);
>> >
>> > That should probably be:
>> >        memcpy(ft1000dev->tx_buf, *pUcFile, word_length * 4);
>> No this shouldn't because before you have additional check:
>> if (byte_length && ((byte_length % 64) == 0))
>>         byte_length += 4;
>>
>> if (byte_length < 64)
>>         byte_length = 68;
>> So in my opinion byte_length should stay.
>
> Yes.  We make byte_length longer than the caller intended.  The caller
> knows the size of the source buffer.  We have to pad the length of the
> other buffer, but we should fill up the last part with zeroes instead
> of reading past the end of the source buffer.
>
> (I am not very familiar with the code and I haven't looked outside this
> function, so I may be wrong).
>
> Also I really bet that the thing where byte_length can't be a multiple
> of 64 is bogus.  I've never heard of anything with a requirement like
> that.
Well I test it and it seems very strange and can't figure out why.
Will remove all byte_length manipulations and device doesn't boot properly
(finish with error). Add some prinkt to code figure out following:

1. byte_length = word_length *4 is < 64 we need to send via usb 68
bytes (despite
4 bytes are behind 64 (without this it can't boot).

2. also when e.g. word_length is 400 (400*4 = 1600) condition
1600%64 == 0 is valid and we send 1604 bytes to usb (also not sure why
but without this change it also doesn't work).

For little explanation when we get to state code load we ask device
how many bytes should send
so we will get reply and send block via usb. So maybe it's related
that we send in our assumption correct data
but usb request something else to properly working ;)
>
> regards,
> dan carpenter
>
>
>

thanks,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
icq: 290551086
web: http://open-nandra.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/