[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <18786.1360763220@warthog.procyon.org.uk>
Date:	Wed, 13 Feb 2013 13:47:00 +0000
From:	David Howells <dhowells@...hat.com>
To:	"Lee, Chun-Yi" <joeyli.kernel@...il.com>
Cc:	dhowells@...hat.com, rusty@...tcorp.com.au,
	linux-kernel@...r.kernel.org, "Lee, Chun-Yi" <jlee@...e.com>,
	Josh Boyer <jwboyer@...hat.com>,
	Randy Dunlap <rdunlap@...otime.net>,
	Herbert Xu <herbert@...dor.hengli.com.au>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH] X.509: Support parse long form of length octets in Authority Key Identifier
Lee, Chun-Yi <joeyli.kernel@...il.com> wrote:
> Signed-off-by: Lee, Chun-Yi <jlee@...e.com>
Without the comma, please.
> +		/* Short Form length */
> +		if (vlen <= 127) {
> +
> +			if (v[1] != vlen - 2 ||
There's an unnecessary blank line there.  I would also move the comment inside
the if-body.
> +			int sub = v[1] - 0x80;
I recommend you use an unsigned int or size_t variable.
Also, you should use ASN1_INDEFINITE_LENGTH rather than writing 0x80 and 127.
> +			v += 4;
> +			key_len = v[3];
Are you sure that is correct?  You altered v before doing v[3].  I would stick
with key_len = vlen - 4.
> +			/* calculate the length from subsequent octet */
"... octets".
> +				seq_len |= v[2 + i];
Add 2 to v before entering the loop.
> +			/* check vlen should not less then length of keyid */
vlen should be exactly equal to key id, shouldn't it?  Leastways, that's what
you're checking...
> +			v += (4 + sub);
> +			key_len = v[3 + sub];
Again, this doesn't look right.  Besides, you should be able to work out
key_len from vlen, subtracting the metadata size.
David
--
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/
Powered by blists - more mailing lists
 
