Skip to contentSkip to conversation

lockd: fix arg parsing for grace_period and timeout.

PATCH Myklebust, Trond 3 participants 3 messages Feb 7, 2012 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

T24gVHVlLCAyMDEyLTAyLTA3IGF0IDE1OjM1ICsxMTAwLCBOZWlsQnJvd24gd3JvdGU6DQo+IElm IHlvdSB0cnkgdG8gc2V0IGdyYWNlX3BlcmlvZCBvciB0aW1lb3V0IHZpYSBhIG1vZHVsZSBwYXJh bWV0ZXINCj4gdG8gbG9ja2QsIGFuZCBkbyB0aGlzIG9uIGEgYmlnLWVuZGlhbiBtYWNoaW5lIHdo ZXJlDQo+IA0KPiAgICBzaXplb2YoaW50KSAhPSBzaXplb2YodW5zaWduZWQgbG9uZykNCj4gDQo+ IGl0IHdvbid0IHdvcmsuICBUaGlzIG51bWJlciBnaXZlbiB3aWxsIGJlIGVmZmVjdGl2ZWx5IHNo aWZ0ZWQgcmlnaHQNCj4gYnkgdGhlIGRpZmZlcmVuY2UgaW4gdGhvc2UgdHdvIHNpemVzLg0KPiAN Cj4gU28gY2FzdCBrcC0+YXJnIHByb3Blcmx5IHRvIGdldCBjb3JyZWN0IHJlc3VsdC4NCj4gDQo+ IFNpZ25lZC1vZmYtYnk6IE5laWxCcm93biA8bmVpbGJAc3VzZS5kZT4NCj4gDQo+IGRpZmYgLS1n aXQgYS9mcy9sb2NrZC9zdmMuYyBiL2ZzL2xvY2tkL3N2Yy5jDQo+IGluZGV4IGMwNjFiOWEuLjI0 NDQ3ODAgMTAwNjQ0DQo+IC0tLSBhL2ZzL2xvY2tkL3N2Yy5jDQo+ICsrKyBiL2ZzL2xvY2tkL3N2 Yy5jDQo+IEBAIC00NDAsNyArNDQwLDcgQEAgc3RhdGljIGludCBwYXJhbV9zZXRfIyNuYW1lKGNv bnN0IGNoYXIgKnZhbCwgc3RydWN0IGtlcm5lbF9wYXJhbSAqa3ApCVwNCj4gIAlfX3R5cGVvZl9f KHR5cGUpIG51bSA9IHdoaWNoX3N0cnRvbCh2YWwsICZlbmRwLCAwKTsJCVwNCj4gIAlpZiAoZW5k cCA9PSB2YWwgfHwgKmVuZHAgfHwgbnVtIDwgKG1pbikgfHwgbnVtID4gKG1heCkpCQlcDQo+ICAJ CXJldHVybiAtRUlOVkFMOwkJCQkJCVwNCj4gLQkqKChpbnQgKikga3AtPmFyZykgPSBudW07CQkJ CQlcDQo+ICsJKigodHlwZSAqKSBrcC0+YXJnKSA9IG51bTsJCQkJCVwNCj4gIAlyZXR1cm4gMDsJ CQkJCQkJXA0KPiAgfQ0KPiAgDQpXb3csIHRoYXQncyBhIHNlcmlvdXNseSBvbGQgYnVnLi4uIEJy dWNlLCBkbyB5b3Ugd2FudCB0byBjYXJyeSBpdD8NCg0KVHJvbmQNCi0tIA0KVHJvbmQgTXlrbGVi dXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1 c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K

#2 lore raw
Quoted 13 lines · Trond
Quoted 12 lines · NeilBrown

If you try to set grace_period or timeout via a module parameter to lockd, and do this on a big-endian machine where

   sizeof(int) != sizeof(unsigned long)

it won't work. This number given will be effectively shifted right by the difference in those two sizes.

So cast kp->arg properly to get correct result.

Quoted 14 lines

fs/lockd/svc.c static int param_set_##name(const char *val, struct kernel_param *kp) \ · line 446

Quoted 13 lines
fs/lockd/svc.c +1 -1
index c061b9a..2444780 100644
@@ -440,7 +440,7 @@ static int param_set_##name(const char *val, struct kernel_param *kp)	\
 	__typeof__(type) num = which_strtol(val, &endp, 0);		\
 	if (endp == val || *endp || num < (min) || num > (max))		\
 		return -EINVAL;						\
-	*((int *) kp->arg) = num;					\
+	*((type *) kp->arg) = num;					\
 	return 0;							\
 }
 

Wow, that's a seriously old bug...

Looking at a historical git repo, it was probably from "[PATCH] make NFS lockd port numbers assignable at run time", attributed to Jamie Lokier in August 2003, in 2.6.0.

Anyway:

Quoted 1 line

Bruce, do you want to carry it?

Sure. I'm a little slow getting stuff pushed out while I track down a couple very tricky bugs, but it should show up in my git tree in a few days....

--b.

#3 lore raw
Quoted 33 lines · On Tue, 7 Feb 2012 09:49:56 -0500 "J.Bruce Fields" <[email protected]> wrote:
Quoted 13 lines · Trond
Quoted 12 lines · NeilBrown

If you try to set grace_period or timeout via a module parameter to lockd, and do this on a big-endian machine where

   sizeof(int) != sizeof(unsigned long)

it won't work. This number given will be effectively shifted right by the difference in those two sizes.

So cast kp->arg properly to get correct result.

Quoted 14 lines

fs/lockd/svc.c static int param_set_##name(const char *val, struct kernel_param *kp) \ · line 446

Quoted 13 lines
fs/lockd/svc.c +1 -1
index c061b9a..2444780 100644
@@ -440,7 +440,7 @@ static int param_set_##name(const char *val, struct kernel_param *kp)	\
 	__typeof__(type) num = which_strtol(val, &endp, 0);		\
 	if (endp == val || *endp || num < (min) || num > (max))		\
 		return -EINVAL;						\
-	*((int *) kp->arg) = num;					\
+	*((type *) kp->arg) = num;					\
 	return 0;							\
 }
 

Wow, that's a seriously old bug...

Looking at a historical git repo, it was probably from "[PATCH] make NFS lockd port numbers assignable at run time", attributed to Jamie Lokier in August 2003, in 2.6.0.

Quoted 7 lines

Anyway:

Quoted 1 line

Bruce, do you want to carry it?

Sure. I'm a little slow getting stuff pushed out while I track down a couple very tricky bugs, but it should show up in my git tree in a few days....

Thanks.. just goes to show how well big-endian is tested I guess :-) especially the more obscure options.

NeilBrown