linux-kernel - Re: [PATCH v2] i2c: stub: Add support for SMBus block commands

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]

Message-ID: <20140708215453.0677d3ed@endymion.delvare>
Date:	Tue, 8 Jul 2014 21:54:53 +0200
From:	Jean Delvare <jdelvare@...e.de>
To:	Guenter Roeck <linux@...ck-us.net>
Cc:	Wolfram Sang <wsa@...-dreams.de>,
	Randy Dunlap <rdunlap@...radead.org>,
	linux-i2c@...r.kernel.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] i2c: stub: Add support for SMBus block commands

Hi Guenter,

On Mon,  7 Jul 2014 07:23:03 -0700, Guenter Roeck wrote:
> SMBus block commands are different to I2C block commands since
> the returned data is not normally accessible with byte or word
> commands on other command offsets. Add linked list of 'block'
> commands to support those commands.
> 
> Access mechanism is quite simple: Block commands must be written
> before they can be read. The first write selects the block length.
> Subsequent writes can be partial. Block read commands always return
> the number of bytes selected with the first write.
> 
> Signed-off-by: Guenter Roeck <linux@...ck-us.net>
> ---
> v2: Make new functionality only available on request via functionality
>     module parameter
>     Add more details about SMBus block mode support to documentation
>     Use correct sizeof() variable in devm_kzalloc
>     Use stub_find_block() only in SMBus block command itself.
>     Store first word of block data in chip->words[].
>     When writing block data and the written data is longer than
>     the first write, bail out with debug message indicating the reason
>     for the error.

Looks good, thanks for the quick update.

Reviewed-by: Jean Delvare <jdelvare@...e.de>

Just one thing I have been thinking about while reviewing the updated
code... You decided to make the first SMBus block write select the
maximum block length, and you always use that for SMBus block reads.
However you accept partial writes. The fact that the order in which
writes are performed has an effect on which writes are accepted is
somewhat unexpected.

Wouldn't it make more sense to accept all SMBus block writes,
regardless of the size (as long as it is within the limits of the SMBus
standard, of course)? Then the only thing left to decide is whether
SMBus block reads use the maximum size or the size of the most recent
SMBus block write.

I suspect this would mimic the behavior of real chips better. What do
you think?

-- 
Jean Delvare
SUSE L3 Support
--
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/