2011-10-19 00:52:13

by Benny Halevy

[permalink] [raw]
Subject: [PATCH] pnfsd: dlm: fix bug in DS tcp/tcp6 address string

squash into "pnfsd: Correctly set netid to tcp or tcp6 for non-local exports"

Currently, the code looks for ':' using strcspn that requires a null terminated string
while the address buffer isn't null terminated.

Instead, just use strnchr to look for a ':' and if not found assume
it's an IPv4 address, otherwise it's IPv6

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfsd/nfs4pnfsdlm.c | 9 ++-------
1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
index abc4d83..d07a6037 100644
--- a/fs/nfsd/nfs4pnfsdlm.c
+++ b/fs/nfsd/nfs4pnfsdlm.c
@@ -328,13 +328,8 @@ static int nfsd4_pnfs_dlm_getdevinfo(struct super_block *sb,
memcpy(daddr->r_addr.data + len, ".8.1", 4);
daddr->r_addr.len = len + 4;

- if (strcspn(daddr->r_addr.data, ":") - 1 == daddr->r_addr.len) {
- daddr->r_netid.data = "tcp";
- daddr->r_netid.len = 3;
- } else {
- daddr->r_netid.data = "tcp6";
- daddr->r_netid.len = 4;
- }
+ daddr->r_netid.data = "tcp6";
+ daddr->r_netid.len = strnchr(daddr->r_addr.data, len, ':') ? 4 : 3;

fdev.fl_device_list[i].fl_multipath_length = 1;
fdev.fl_device_list[i].fl_multipath_list = daddr;
--
1.7.6



2011-10-31 21:16:43

by Olga Kornievskaia

[permalink] [raw]
Subject: Re: [PATCH] pnfsd: dlm: fix bug in DS tcp/tcp6 address string

Benny, this patch does not fix the issue. Except that now all the
entries list "tcp6". Previously if you recall first data server entry
was correct but others listed tcp6 type with it was suppose to be tcp.

On Tue, Oct 18, 2011 at 8:52 PM, Benny Halevy <[email protected]> wrote:
> squash into "pnfsd: Correctly set netid to tcp or tcp6 for non-local exports"
>
> Currently, the code looks for ':' using strcspn that requires a null terminated string
> while the address buffer isn't null terminated.
>
> Instead, just use strnchr to look for a ':' and if not found assume
> it's an IPv4 address, otherwise it's IPv6
>
> Signed-off-by: Benny Halevy <[email protected]>
> ---
> ?fs/nfsd/nfs4pnfsdlm.c | ? ?9 ++-------
> ?1 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
> index abc4d83..d07a6037 100644
> --- a/fs/nfsd/nfs4pnfsdlm.c
> +++ b/fs/nfsd/nfs4pnfsdlm.c
> @@ -328,13 +328,8 @@ static int nfsd4_pnfs_dlm_getdevinfo(struct super_block *sb,
> ? ? ? ? ? ? ? ?memcpy(daddr->r_addr.data + len, ".8.1", 4);
> ? ? ? ? ? ? ? ?daddr->r_addr.len = len + 4;
>
> - ? ? ? ? ? ? ? if (strcspn(daddr->r_addr.data, ":") - 1 == daddr->r_addr.len) {
> - ? ? ? ? ? ? ? ? ? ? ? daddr->r_netid.data = "tcp";
> - ? ? ? ? ? ? ? ? ? ? ? daddr->r_netid.len = 3;
> - ? ? ? ? ? ? ? } else {
> - ? ? ? ? ? ? ? ? ? ? ? daddr->r_netid.data = "tcp6";
> - ? ? ? ? ? ? ? ? ? ? ? daddr->r_netid.len = 4;
> - ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? daddr->r_netid.data = "tcp6";
> + ? ? ? ? ? ? ? daddr->r_netid.len = strnchr(daddr->r_addr.data, len, ':') ? 4 : 3;
>
> ? ? ? ? ? ? ? ?fdev.fl_device_list[i].fl_multipath_length = 1;
> ? ? ? ? ? ? ? ?fdev.fl_device_list[i].fl_multipath_list = daddr;
> --
> 1.7.6
>
>

2011-10-19 16:08:10

by Weston Andros Adamson

[permalink] [raw]
Subject: Re: [PATCH] pnfsd: dlm: fix bug in DS tcp/tcp6 address string

Looks good to me.

-dros

On Oct 18, 2011, at 8:52 PM, Benny Halevy wrote:

> squash into "pnfsd: Correctly set netid to tcp or tcp6 for non-local exports"
>
> Currently, the code looks for ':' using strcspn that requires a null terminated string
> while the address buffer isn't null terminated.
>
> Instead, just use strnchr to look for a ':' and if not found assume
> it's an IPv4 address, otherwise it's IPv6
>
> Signed-off-by: Benny Halevy <[email protected]>
> ---
> fs/nfsd/nfs4pnfsdlm.c | 9 ++-------
> 1 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
> index abc4d83..d07a6037 100644
> --- a/fs/nfsd/nfs4pnfsdlm.c
> +++ b/fs/nfsd/nfs4pnfsdlm.c
> @@ -328,13 +328,8 @@ static int nfsd4_pnfs_dlm_getdevinfo(struct super_block *sb,
> memcpy(daddr->r_addr.data + len, ".8.1", 4);
> daddr->r_addr.len = len + 4;
>
> - if (strcspn(daddr->r_addr.data, ":") - 1 == daddr->r_addr.len) {
> - daddr->r_netid.data = "tcp";
> - daddr->r_netid.len = 3;
> - } else {
> - daddr->r_netid.data = "tcp6";
> - daddr->r_netid.len = 4;
> - }
> + daddr->r_netid.data = "tcp6";
> + daddr->r_netid.len = strnchr(daddr->r_addr.data, len, ':') ? 4 : 3;
>
> fdev.fl_device_list[i].fl_multipath_length = 1;
> fdev.fl_device_list[i].fl_multipath_list = daddr;
> --
> 1.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


Attachments:
smime.p7s (1.34 kB)

2011-11-02 11:33:13

by Benny Halevy

[permalink] [raw]
Subject: Re: [PATCH] pnfsd: dlm: fix bug in DS tcp/tcp6 address string

SGksIHRoYW5rcyBmb3IgdGVzdGluZyB0aGUgcGF0Y2ghDQpEbyBhbGwgZW50cmllcyBoYXZlIHJf
bmV0aWQubGVuPT00Pw0KQ2FuIHlvdSBwbGVhc2UgcHJpbnQgcl9hZGRyLmRhdGEgYW5kIHJfYWRk
ci5sZW4/DQoNCkJlbm55DQoNCi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpGcm9tOiBPbGdh
IEtvcm5pZXZza2FpYSA8YWdsb0BjaXRpLnVtaWNoLmVkdT4NClNlbmRlcjogb2xnYS5rb3JuaWV2
c2thaWFAZ21haWwuY29tDQpEYXRlOiBNb24sIDMxIE9jdCAyMDExIDE3OjE2OjQxIA0KVG86IEJl
bm55IEhhbGV2eTxiaGFsZXZ5QHRvbmlhbi5jb20+DQpDYzogTWljaGFlbCBHcm9zaGFuczxncm9z
aGFuc0B1bWljaC5lZHU+OyBORlMgbGlzdDxsaW51eC1uZnNAdmdlci5rZXJuZWwub3JnPg0KU3Vi
amVjdDogUmU6IFtQQVRDSF0gcG5mc2Q6IGRsbTogZml4IGJ1ZyBpbiBEUyB0Y3AvdGNwNiBhZGRy
ZXNzIHN0cmluZw0KDQpCZW5ueSwgdGhpcyBwYXRjaCBkb2VzIG5vdCBmaXggdGhlIGlzc3VlLiBF
eGNlcHQgdGhhdCBub3cgYWxsIHRoZQ0KZW50cmllcyBsaXN0ICJ0Y3A2Ii4gUHJldmlvdXNseSBp
ZiB5b3UgcmVjYWxsIGZpcnN0IGRhdGEgc2VydmVyIGVudHJ5DQp3YXMgY29ycmVjdCBidXQgb3Ro
ZXJzIGxpc3RlZCB0Y3A2IHR5cGUgd2l0aCBpdCB3YXMgc3VwcG9zZSB0byBiZSB0Y3AuDQoNCk9u
IFR1ZSwgT2N0IDE4LCAyMDExIGF0IDg6NTIgUE0sIEJlbm55IEhhbGV2eSA8YmhhbGV2eUB0b25p
YW4uY29tPiB3cm90ZToNCj4gc3F1YXNoIGludG8gInBuZnNkOiBDb3JyZWN0bHkgc2V0IG5ldGlk
IHRvIHRjcCBvciB0Y3A2IGZvciBub24tbG9jYWwgZXhwb3J0cyINCj4NCj4gQ3VycmVudGx5LCB0
aGUgY29kZSBsb29rcyBmb3IgJzonIHVzaW5nIHN0cmNzcG4gdGhhdCByZXF1aXJlcyBhIG51bGwg
dGVybWluYXRlZCBzdHJpbmcNCj4gd2hpbGUgdGhlIGFkZHJlc3MgYnVmZmVyIGlzbid0IG51bGwg
dGVybWluYXRlZC4NCj4NCj4gSW5zdGVhZCwganVzdCB1c2Ugc3RybmNociB0byBsb29rIGZvciBh
ICc6JyBhbmQgaWYgbm90IGZvdW5kIGFzc3VtZQ0KPiBpdCdzIGFuIElQdjQgYWRkcmVzcywgb3Ro
ZXJ3aXNlIGl0J3MgSVB2Ng0KPg0KPiBTaWduZWQtb2ZmLWJ5OiBCZW5ueSBIYWxldnkgPGJoYWxl
dnlAdG9uaWFuLmNvbT4NCj4gLS0tDQo+IKBmcy9uZnNkL25mczRwbmZzZGxtLmMgfCCgIKA5ICsr
LS0tLS0tLQ0KPiCgMSBmaWxlcyBjaGFuZ2VkLCAyIGluc2VydGlvbnMoKyksIDcgZGVsZXRpb25z
KC0pDQo+DQo+IGRpZmYgLS1naXQgYS9mcy9uZnNkL25mczRwbmZzZGxtLmMgYi9mcy9uZnNkL25m
czRwbmZzZGxtLmMNCj4gaW5kZXggYWJjNGQ4My4uZDA3YTYwMzcgMTAwNjQ0DQo+IC0tLSBhL2Zz
L25mc2QvbmZzNHBuZnNkbG0uYw0KPiArKysgYi9mcy9uZnNkL25mczRwbmZzZGxtLmMNCj4gQEAg
LTMyOCwxMyArMzI4LDggQEAgc3RhdGljIGludCBuZnNkNF9wbmZzX2RsbV9nZXRkZXZpbmZvKHN0
cnVjdCBzdXBlcl9ibG9jayAqc2IsDQo+IKAgoCCgIKAgoCCgIKAgoG1lbWNweShkYWRkci0+cl9h
ZGRyLmRhdGEgKyBsZW4sICIuOC4xIiwgNCk7DQo+IKAgoCCgIKAgoCCgIKAgoGRhZGRyLT5yX2Fk
ZHIubGVuID0gbGVuICsgNDsNCj4NCj4gLSCgIKAgoCCgIKAgoCCgIGlmIChzdHJjc3BuKGRhZGRy
LT5yX2FkZHIuZGF0YSwgIjoiKSAtIDEgPT0gZGFkZHItPnJfYWRkci5sZW4pIHsNCj4gLSCgIKAg
oCCgIKAgoCCgIKAgoCCgIKAgZGFkZHItPnJfbmV0aWQuZGF0YSA9ICJ0Y3AiOw0KPiAtIKAgoCCg
IKAgoCCgIKAgoCCgIKAgoCBkYWRkci0+cl9uZXRpZC5sZW4gPSAzOw0KPiAtIKAgoCCgIKAgoCCg
IKAgfSBlbHNlIHsNCj4gLSCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgZGFkZHItPnJfbmV0aWQuZGF0
YSA9ICJ0Y3A2IjsNCj4gLSCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgZGFkZHItPnJfbmV0aWQubGVu
ID0gNDsNCj4gLSCgIKAgoCCgIKAgoCCgIH0NCj4gKyCgIKAgoCCgIKAgoCCgIGRhZGRyLT5yX25l
dGlkLmRhdGEgPSAidGNwNiI7DQo+ICsgoCCgIKAgoCCgIKAgoCBkYWRkci0+cl9uZXRpZC5sZW4g
PSBzdHJuY2hyKGRhZGRyLT5yX2FkZHIuZGF0YSwgbGVuLCAnOicpID8gNCA6IDM7DQo+DQo+IKAg
oCCgIKAgoCCgIKAgoGZkZXYuZmxfZGV2aWNlX2xpc3RbaV0uZmxfbXVsdGlwYXRoX2xlbmd0aCA9
IDE7DQo+IKAgoCCgIKAgoCCgIKAgoGZkZXYuZmxfZGV2aWNlX2xpc3RbaV0uZmxfbXVsdGlwYXRo
X2xpc3QgPSBkYWRkcjsNCj4gLS0NCj4gMS43LjYNCj4NCj4NCg==