linux-kernel - Re: [PATCH 1/3] Drivers: hv: Support the newly introduced KVP messages in the driver

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] [day] [month] [year] [list]

Message-ID: <20120316142649.GJ3163@mwanda>
Date:	Fri, 16 Mar 2012 17:26:49 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	KY Srinivasan <kys@...rosoft.com>
Cc:	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"ohering@...e.com" <ohering@...e.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"virtualization@...ts.osdl.org" <virtualization@...ts.osdl.org>,
	Alan Stern <stern@...land.harvard.edu>,
	"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>
Subject: Re: [PATCH 1/3] Drivers: hv: Support the newly introduced KVP
 messages in the driver

On Fri, Mar 16, 2012 at 01:14:07PM +0000, KY Srinivasan wrote:
> Dan,
> I am trying to understand your concern here:
> You will agree with me that the current code does not overflow the 
> buffer since the output is limited to MAX bytes and that is how 
> big the output buffer is sized. Now, as I pointed out earlier, if the
> string to be converted were to fully occupy the MAX bytes or even be
> larger than MAX bytes (no buffer overflow here since we limit the
> conversion to MAX bytes), we will get a malformed packet that will be
> sent to the host since the size field of the message would exceed
> the protocol specified size limit. I was merely pointing out that in
> this case, I would rather have the host reject the message than send
> a truncated Key/Value string (if we were to ever get invalid key or
> value strings). 
> 

Sending malformed packets is a bad idea.

Normally, if you handle the error as close to the cause of the error
as possible then it makes everything easier to read.  In this case
especially, it's so easy to catch any errors.  If you decide to go
ahead and send the malformed message, at least put a comment.

When I read code, I spend all my time looking for what it does
wrong.  So when code doesn't have any error handling and sets
keylen = -EINVAL then sure, it's fewer lines of code to read, but
it doesn't make my life easier.  Readable code has obvious error
handling.

Introducing off by one errors is an especially bad idea.  It doesn't
matter how harmless they are, because they end up getting copy and
pasted.  I don't know how to make it any clearer than that.  :/
Never never never do that!

regards,
dan carpenter

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)