James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: > > Didn't you say you were going to make it return an error when it ran > > out of space or was asked to encode a negative number? > > it follows the pattern of all the other functions in that it dumps a > kernel warning on problems and bails. I don't see any bounds checking there, let alone anything that dumps a kernel warning and bails - except if the length is so large that the ASN.1 cannot be constructed. That said, there is a check in patch 4. > ... as long as the buffer doesn't overflow. Yes, but that's Dave's point. [from patch 4] > + * Assume both ovtet strings will encode to a 2 byte definite length octet, btw. At least I've found a preliminary bounds check there > + */ > + if (WARN(work - scratch + pub_len + priv_len + 8 > SCRATCH_SIZE, > + "BUG: scratch buffer is too small")) > + return -EINVAL; which I presume makes the correct calculation. I thought TPM comms were slow - slow enough that putting bounds checking in your asn1_encode_* routines will be insignificant. Further, you're promoting this ASN.1 encoder as a general library within the kernel, presumably so that other people can make use of it. Please therefore put bounds checking and error handling in it. And please *don't* just produce broken ASN.1 when something goes wrong. David
