[Open-FCoE] [PATCH] fcoeadm: --target segfault with other FC storage present

[Chris Leech]

fcoeadm is segfaulting when trying to parse sysfs paths to rports for
traditional FC or non fcoe-utils/libfc FCoE.

I also changed search_rports to just ignore these instead of returning
an error, so that the target information for all fcoe-utils managed
storage can be displayed instead of stopping at the first exception.

Signed-off-by: Chris Leech <***@redhat.com>
---
fcoeadm_display.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fcoeadm_display.c b/fcoeadm_display.c
index 7b95aa4..bf3831a 100644
--- a/fcoeadm_display.c
+++ b/fcoeadm_display.c
@@ -660,17 +660,19 @@ static char *get_ifname_from_rport(char *rport)

err = asprintf(&path, "%s/%s", "/sys/class/fc_remote_ports", rport);
if (err == -1)
- return false;
+ return NULL;

ret = readlink(path, link, sizeof(link));
free(path);
if (ret == -1)
- return false;
+ return NULL;

if (link[ret] != '\0')
link[ret] = '\0';

offs = strstr(link, "/net/");
+ if (!offs)
+ return NULL;

offs = offs + 5;

@@ -778,7 +780,7 @@ static int search_rports(struct dirent *dp, void *arg)
} else {
ifname = get_ifname_from_rport(rport);
if (!ifname)
- return -ENOMEM;
+ return 0;
allocated = true;
}

[Johannes Thumshirn]

Post by Chris Leech
fcoeadm is segfaulting when trying to parse sysfs paths to rports for
traditional FC or non fcoe-utils/libfc FCoE.
I also changed search_rports to just ignore these instead of returning
an error, so that the target information for all fcoe-utils managed
storage can be displayed instead of stopping at the first exception.
---
fcoeadm_display.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/fcoeadm_display.c b/fcoeadm_display.c
index 7b95aa4..bf3831a 100644
--- a/fcoeadm_display.c
+++ b/fcoeadm_display.c
@@ -660,17 +660,19 @@ static char *get_ifname_from_rport(char *rport)
err = asprintf(&path, "%s/%s", "/sys/class/fc_remote_ports", rport);
if (err == -1)
- return false;
+ return NULL;
ret = readlink(path, link, sizeof(link));
free(path);
if (ret == -1)
- return false;
+ return NULL;
if (link[ret] != '\0')
link[ret] = '\0';
offs = strstr(link, "/net/");
+ if (!offs)
+ return NULL;
offs = offs + 5;
@@ -778,7 +780,7 @@ static int search_rports(struct dirent *dp, void *arg)
} else {
ifname = get_ifname_from_rport(rport);
if (!ifname)
- return -ENOMEM;
+ return 0;
allocated = true;
}
--
2.7.4

Hi Chris,

Looks good but I noticed we're leaking memory here (from the asprintf()
calls). I know it's my code that leaks it but do you mind fixing it (for
get_ifname_from_rport()) while you're at it?

Thanks,
Johannes

[Chris Leech]

Post by Johannes Thumshirn
Hi Chris,
Looks good but I noticed we're leaking memory here (from the asprintf()
calls). I know it's my code that leaks it but do you mind fixing it (for
get_ifname_from_rport()) while you're at it?

Sorry, but I'm not seeing it. If asprintf allocates memory, it's used
as an argument to readlink and then freed. What am I missing?

Chris

[Johannes Thumshirn]

Post by Chris Leech

Post by Johannes Thumshirn
Hi Chris,
Looks good but I noticed we're leaking memory here (from the asprintf()
calls). I know it's my code that leaks it but do you mind fixing it (for
get_ifname_from_rport()) while you're at it?

Sorry, but I'm not seeing it. If asprintf allocates memory, it's used
as an argument to readlink and then freed. What am I missing?

You're right, I totally missed the call to free. Must've been too early in the
morning. And I reacll I did a valgrind run each test so I should have spotted
eventual leaks...

I'm sorry. Not a good week for me.

Anyways, thanks for the patch. I'll apply it and push to my github as I still
don't have commit access to open-fcoe.org's git :-/.

While we're at it, do you guys have any foce-utils patches in the queue? I'd
like to push out a new release soon (say after RHEL 7.3 and SLES12SP2 are out)?

Thanks,
Johannes

[Chris Leech]

Post by Johannes Thumshirn
Anyways, thanks for the patch. I'll apply it and push to my github as I still
don't have commit access to open-fcoe.org's git :-/.
While we're at it, do you guys have any foce-utils patches in the queue? I'd
like to push out a new release soon (say after RHEL 7.3 and SLES12SP2 are out)?

Looks like I've only got one out of tree patch, dealing with the fact
that at some point a SAN MAC became required when it should be optional.

---

From: Chris Leech <***@redhat.com>
Date: Thu, 5 Feb 2015 11:46:31 -0800
Subject: [PATCH] sanmac isn't required

Signed-off-by: Chris Leech <***@redhat.com>
---
lib/fip.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/fip.c b/lib/fip.c
index 6657b61..5e5b179 100644
--- a/lib/fip.c
+++ b/lib/fip.c
@@ -215,7 +215,7 @@ int fip_socket(int ifindex, enum fip_multi multi)
return s;

rc = fip_socket_sanmac(s, ifindex, 1);
- if (rc < 0) {
+ if (rc < 0 && rc != -ENXIO) {
close(s);
return rc;
}

[Johannes Thumshirn]

Post by Chris Leech

Post by Johannes Thumshirn
Anyways, thanks for the patch. I'll apply it and push to my github as I still
don't have commit access to open-fcoe.org's git :-/.
While we're at it, do you guys have any foce-utils patches in the queue? I'd
like to push out a new release soon (say after RHEL 7.3 and SLES12SP2 are out)?

Looks like I've only got one out of tree patch, dealing with the fact
that at some point a SAN MAC became required when it should be optional.

I can queue that one up as well if you want. I'll also see what we're carrying
around with our distros.

Thanks,
Johannes

Post by Chris Leech
---
Date: Thu, 5 Feb 2015 11:46:31 -0800
Subject: [PATCH] sanmac isn't required
---
lib/fip.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/fip.c b/lib/fip.c
index 6657b61..5e5b179 100644
--- a/lib/fip.c
+++ b/lib/fip.c
@@ -215,7 +215,7 @@ int fip_socket(int ifindex, enum fip_multi multi)
return s;
rc = fip_socket_sanmac(s, ifindex, 1);
- if (rc < 0) {
+ if (rc < 0 && rc != -ENXIO) {
close(s);
return rc;
}
--
2.1.0

[Johannes Thumshirn]

Post by Chris Leech

Post by Johannes Thumshirn
Anyways, thanks for the patch. I'll apply it and push to my github as I still
don't have commit access to open-fcoe.org's git :-/.
While we're at it, do you guys have any foce-utils patches in the queue? I'd
like to push out a new release soon (say after RHEL 7.3 and SLES12SP2 are out)?

Looks like I've only got one out of tree patch, dealing with the fact
that at some point a SAN MAC became required when it should be optional.

Both patches applied and pushed out to:
***@github.com:morbidrsa/fcoe-utils.git


Thanks,
Johannes