2012-03-11 22:14:46

by Myklebust, Trond

[permalink] [raw]
Subject: Bug in gss_krb5_crypto.c

SGkgS2V2aW4gJiBCcnVjZSwNCg0KV2hlbiBydW5uaW5nIHNwYXJzZSBvbiB0aGUgc3VucnBjIGNv
ZGUsIEknbSBzZWVpbmcgdGhlIGZvbGxvd2luZyBlcnJvcg0KbWVzc2FnZToNCiJuZXQvc3VucnBj
L2F1dGhfZ3NzL2dzc19rcmI1X2NyeXB0by5jOjYwMzo1MjogZXJyb3I6IGJhZCBjb25zdGFudA0K
ZXhwcmVzc2lvbiINCg0KVGhpcyBib2lscyBkb3duIHRvIHRoZSBsaW5lOg0KICAgICAgICB1OCBk
YXRhW2NyeXB0b19ibGtjaXBoZXJfYmxvY2tzaXplKGNpcGhlcikgKiAyXTsNCg0Kd2hpY2ggaXMg
dXNpbmcgYSBHTlUgQ2MtaXNtIGluIG9yZGVyIHRvIGRvIGR5bmFtaWMgYWxsb2NhdGlvbiBvbiB0
aGUNCnN0YWNrLiBUaGlzIGlzIG5vdCBhY2NlcHRhYmxlIGluIGtlcm5lbCBjb2RlLCBhbmQgcmVh
bGx5IG5lZWRzIHRvIGJlDQpjaGFuZ2VkLiBIb3cgc2hvdWxkIHdlIG1vdmUgZm9yd2FyZCB3aXRo
IHRoaXM/IElzIHRoZXJlIGFuIHVwcGVyIGxpbWl0DQpvbiB0aGUgdmFsdWUgb2YgY3J5cHRvX2Js
a2NpcGhlcl9ibG9ja3NpemUoY2lwaGVyKSB0aGF0IHdlIGNhbiB1c2U/DQoNCkNoZWVycw0KICBU
cm9uZA0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVy
DQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoN
Cg==


2012-03-12 15:13:54

by Kevin Coffman

[permalink] [raw]
Subject: Re: Bug in gss_krb5_crypto.c

On Sun, Mar 11, 2012 at 7:02 PM, Dr James Bruce Fields
<[email protected]> wrote:
> On Sun, Mar 11, 2012 at 10:14:44PM +0000, Myklebust, Trond wrote:
>> Hi Kevin & Bruce,
>>
>> When running sparse on the sunrpc code, I'm seeing the following error
>> message:
>> "net/sunrpc/auth_gss/gss_krb5_crypto.c:603:52: error: bad constant
>> expression"
>>
>> This boils down to the line:
>> ? ? ? ? u8 data[crypto_blkcipher_blocksize(cipher) * 2];
>
> Oops.
>
>> which is using a GNU Cc-ism in order to do dynamic allocation on the
>> stack. This is not acceptable in kernel code, and really needs to be
>> changed. How should we move forward with this? Is there an upper limit
>> on the value of crypto_blkcipher_blocksize(cipher) that we can use?
>
> I think it's always either 8 or 16.
>
> --b.
>

Yes, I think 16 is currently the upper blocksize limit. I was
thinking there was a constant defined in a header for the
maxblocksize, but I'm not able to get to my machine to grep for it
right now.

K.C.

2012-03-12 17:04:43

by Myklebust, Trond

[permalink] [raw]
Subject: Re: Bug in gss_krb5_crypto.c

T24gTW9uLCAyMDEyLTAzLTEyIGF0IDExOjEzIC0wNDAwLCBLZXZpbiBDb2ZmbWFuIHdyb3RlOg0K
PiBPbiBTdW4sIE1hciAxMSwgMjAxMiBhdCA3OjAyIFBNLCBEciBKYW1lcyBCcnVjZSBGaWVsZHMN
Cj4gPGJmaWVsZHNAZmllbGRzZXMub3JnPiB3cm90ZToNCj4gPiBPbiBTdW4sIE1hciAxMSwgMjAx
MiBhdCAxMDoxNDo0NFBNICswMDAwLCBNeWtsZWJ1c3QsIFRyb25kIHdyb3RlOg0KPiA+PiBIaSBL
ZXZpbiAmIEJydWNlLA0KPiA+Pg0KPiA+PiBXaGVuIHJ1bm5pbmcgc3BhcnNlIG9uIHRoZSBzdW5y
cGMgY29kZSwgSSdtIHNlZWluZyB0aGUgZm9sbG93aW5nIGVycm9yDQo+ID4+IG1lc3NhZ2U6DQo+
ID4+ICJuZXQvc3VucnBjL2F1dGhfZ3NzL2dzc19rcmI1X2NyeXB0by5jOjYwMzo1MjogZXJyb3I6
IGJhZCBjb25zdGFudA0KPiA+PiBleHByZXNzaW9uIg0KPiA+Pg0KPiA+PiBUaGlzIGJvaWxzIGRv
d24gdG8gdGhlIGxpbmU6DQo+ID4+ICAgICAgICAgdTggZGF0YVtjcnlwdG9fYmxrY2lwaGVyX2Js
b2Nrc2l6ZShjaXBoZXIpICogMl07DQo+ID4NCj4gPiBPb3BzLg0KPiA+DQo+ID4+IHdoaWNoIGlz
IHVzaW5nIGEgR05VIENjLWlzbSBpbiBvcmRlciB0byBkbyBkeW5hbWljIGFsbG9jYXRpb24gb24g
dGhlDQo+ID4+IHN0YWNrLiBUaGlzIGlzIG5vdCBhY2NlcHRhYmxlIGluIGtlcm5lbCBjb2RlLCBh
bmQgcmVhbGx5IG5lZWRzIHRvIGJlDQo+ID4+IGNoYW5nZWQuIEhvdyBzaG91bGQgd2UgbW92ZSBm
b3J3YXJkIHdpdGggdGhpcz8gSXMgdGhlcmUgYW4gdXBwZXIgbGltaXQNCj4gPj4gb24gdGhlIHZh
bHVlIG9mIGNyeXB0b19ibGtjaXBoZXJfYmxvY2tzaXplKGNpcGhlcikgdGhhdCB3ZSBjYW4gdXNl
Pw0KPiA+DQo+ID4gSSB0aGluayBpdCdzIGFsd2F5cyBlaXRoZXIgOCBvciAxNi4NCj4gPg0KPiA+
IC0tYi4NCj4gPg0KPiANCj4gWWVzLCBJIHRoaW5rIDE2IGlzIGN1cnJlbnRseSB0aGUgdXBwZXIg
YmxvY2tzaXplIGxpbWl0LiAgSSB3YXMNCj4gdGhpbmtpbmcgdGhlcmUgd2FzIGEgY29uc3RhbnQg
ZGVmaW5lZCBpbiBhIGhlYWRlciBmb3IgdGhlDQo+IG1heGJsb2Nrc2l6ZSwgYnV0IEknbSBub3Qg
YWJsZSB0byBnZXQgdG8gbXkgbWFjaGluZSB0byBncmVwIGZvciBpdA0KPiByaWdodCBub3cuDQoN
CkkgY2FuJ3Qgc2VlIGFuIGFic29sdXRlIG1heGltdW0gZm9yIHRoZSBibG9ja3NpemUuIFRoZSBi
aWdnZXN0IGJsb2Nrc2l6ZQ0KdGhhdCBJIGNhbiBmaW5kIGluIHRoZSBzb3VyY2VzIG5vdyBpcyBT
SEE1MTJfQkxPQ0tfU0laRSwgd2l0aCB0aGUgdmFsdWUNCjEyOC4gVGhhdCBsb29rcyBhIGxpdHRs
ZSBsYXJnZSB0byBiZSB1c2luZyBpbiBhIHN0YWNrIHZhcmlhYmxlLg0KDQotLSANClRyb25kIE15
a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlr
bGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==

2012-03-11 23:02:52

by J. Bruce Fields

[permalink] [raw]
Subject: Re: Bug in gss_krb5_crypto.c

On Sun, Mar 11, 2012 at 10:14:44PM +0000, Myklebust, Trond wrote:
> Hi Kevin & Bruce,
>
> When running sparse on the sunrpc code, I'm seeing the following error
> message:
> "net/sunrpc/auth_gss/gss_krb5_crypto.c:603:52: error: bad constant
> expression"
>
> This boils down to the line:
> u8 data[crypto_blkcipher_blocksize(cipher) * 2];

Oops.

> which is using a GNU Cc-ism in order to do dynamic allocation on the
> stack. This is not acceptable in kernel code, and really needs to be
> changed. How should we move forward with this? Is there an upper limit
> on the value of crypto_blkcipher_blocksize(cipher) that we can use?

I think it's always either 8 or 16.

--b.

2012-03-12 17:13:17

by Kevin Coffman

[permalink] [raw]
Subject: Re: Bug in gss_krb5_crypto.c

On Mon, Mar 12, 2012 at 1:04 PM, Myklebust, Trond
<[email protected]> wrote:
> On Mon, 2012-03-12 at 11:13 -0400, Kevin Coffman wrote:
>> On Sun, Mar 11, 2012 at 7:02 PM, Dr James Bruce Fields
>> <[email protected]> wrote:
>> > On Sun, Mar 11, 2012 at 10:14:44PM +0000, Myklebust, Trond wrote:
>> >> Hi Kevin & Bruce,
>> >>
>> >> When running sparse on the sunrpc code, I'm seeing the following error
>> >> message:
>> >> "net/sunrpc/auth_gss/gss_krb5_crypto.c:603:52: error: bad constant
>> >> expression"
>> >>
>> >> This boils down to the line:
>> >> ? ? ? ? u8 data[crypto_blkcipher_blocksize(cipher) * 2];
>> >
>> > Oops.
>> >
>> >> which is using a GNU Cc-ism in order to do dynamic allocation on the
>> >> stack. This is not acceptable in kernel code, and really needs to be
>> >> changed. How should we move forward with this? Is there an upper limit
>> >> on the value of crypto_blkcipher_blocksize(cipher) that we can use?
>> >
>> > I think it's always either 8 or 16.
>> >
>> > --b.
>> >
>>
>> Yes, I think 16 is currently the upper blocksize limit. ?I was
>> thinking there was a constant defined in a header for the
>> maxblocksize, but I'm not able to get to my machine to grep for it
>> right now.
>
> I can't see an absolute maximum for the blocksize. The biggest blocksize
> that I can find in the sources now is SHA512_BLOCK_SIZE, with the value
> 128. That looks a little large to be using in a stack variable.
>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> [email protected]
> http://www.netapp.com
>

Olga got my machine back online :-)

GSS_KRB5_MAX_BLOCKSIZE is defined in include/linux/sunrpc/gss_krb5.h

The value above is in bytes (16).

K.C.