linux-kernel - Re: [PATCH v4 2/2] tpm: add support for partial reads

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: <20181119135800.GE8755@linux.intel.com>
Date:   Mon, 19 Nov 2018 15:58:00 +0200
From:   Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
To:     Tadeusz Struk <tadeusz.struk@...el.com>
Cc:     jgg@...pe.ca, linux-integrity@...r.kernel.org,
        linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/2] tpm: add support for partial reads

On Sun, Nov 18, 2018 at 07:05:19PM -0800, Tadeusz Struk wrote:
> > When ret_size < 0? Shouldn't this be just "if (!ret_size)"?
> 
> What we want to check here is if ret_size is positive, which is a valid
> value, or if it is negative or zero, which is an invalid value, so in
> this case (!ret_size) will not work.

Please explain a scenario where "!ret_size" would no work given that
both size and partial_data have always positive value?

> >> 	/* Holds the resul of the last successful call to tpm_transmit() */
> >>  	size_t transmit_result;
> >> +	/* Holds the count how much of the response is still unread */
> >> +	size_t partial_data;
> > I'm otherwise happy how this look like but why call it partial_data.
> > You cannot really tell from the name anything about its contents as
> > data is very abstract term.
>  
> so I will rename these two to response_length and response_length_rem,
> how does this sound?

Yes, assuming that there would be a hard requirement to even have two
variables in the first place.

> > BTW, why you need the new variable anyway and not just decrease the
> > variable where the length is original stored?
> 
> We need to have two variables, otherwise how do we tell if some part of
> response was consumed to allow sending a new command?

I don't understand. In order to maintain backwards compatibility you can
send a new command at any time.

> The transmit_result is used for that. If it is zero then one can transmit
> a new command even if the whole response is not consumed. The new variable
> tracks how much of the response is still to be read. 

AFAIK you only need to track the latter, not both.

/Jarkko