On 3/19/20 10:07 PM, Jarkko Sakkinen wrote:

On Thu, Mar 19, 2020 at 08:07:55PM -0400, Waiman Long wrote:

quoted

On 3/19/20 3:46 PM, Jarkko Sakkinen wrote:

quoted

On Wed, Mar 18, 2020 at 06:14:57PM -0400, Waiman Long wrote:

quoted

+			 * It is possible, though unlikely, that the key
+			 * changes in between the up_read->down_read period.
+			 * If the key becomes longer, we will have to
+			 * allocate a larger buffer and redo the key read
+			 * again.
+			 */
+			if (!tmpbuf || unlikely(ret > tmpbuflen)) {

Shouldn't you check that tmpbuflen stays below buflen (why else
you had made copy of buflen otherwise)?

The check above this thunk:

if ((ret > 0) && (ret <= buflen)) {

will make sure that ret will not be larger than buflen. So tmpbuflen
will never be bigger than buflen.

Ah right, of course, thanks.

What would go wrong if the condition was instead
((ret > 0) && (ret <= tmpbuflen))?

That if statement is a check to see if the actual key length is longer
than the user-supplied buffer (buflen). If that is the case, it will
just return the expected length without storing anything into the user
buffer. For the case that buflen >= ret > tmpbuflen, the revised check
above will incorrectly skip the storing step causing the caller to
incorrectly think the key is there in the buffer.

Maybe I should clarify that a bit more in the comment.

Cheers,
Longman