Re: [PATCH] lockd: fix arg parsing for grace_period and timeout.

Re: [PATCH] lockd: fix arg parsing for grace_period and timeout.

[Myklebust, Trond]

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

Re: [PATCH] lockd: fix arg parsing for grace_period and timeout.

[J.Bruce Fields]

On Tue, Feb 07, 2012 at 01:58:43PM +0000, Myklebust, Trond wrote:
> On Tue, 2012-02-07 at 15:35 +1100, NeilBrown wrote:
> > 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.
> > 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > 
> > diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> > index c061b9a..2444780 100644
> > --- a/fs/lockd/svc.c
> > +++ b/fs/lockd/svc.c
> > @@ -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:

> 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.

Re: [PATCH] lockd: fix arg parsing for grace_period and timeout.

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1756 bytes --]

On Tue, 7 Feb 2012 09:49:56 -0500 "J.Bruce Fields" <bfields@fieldses.org>
wrote:

> On Tue, Feb 07, 2012 at 01:58:43PM +0000, Myklebust, Trond wrote:
> > On Tue, 2012-02-07 at 15:35 +1100, NeilBrown wrote:
> > > 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.
> > > 
> > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > 
> > > diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> > > index c061b9a..2444780 100644
> > > --- a/fs/lockd/svc.c
> > > +++ b/fs/lockd/svc.c
> > > @@ -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:
> 
> > 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]