xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/2] libxl: replace deprecated readdir_r() with readdir()
@ 2016-06-03 16:50 Chris Patterson
  2016-06-03 16:50 ` [PATCH v3 2/2] libfsimage: " Chris Patterson
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Chris Patterson @ 2016-06-03 16:50 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson, Chris Patterson, wei.liu2

From: Chris Patterson <pattersonc@ainfosec.com>

Replace the usage of readdir_r() with readdir() to address a
compilation error under glibc due to the deprecation of readdir_r
for their next release (2.24) [1, 2].

Remove code specific to usage of readdir_r which is no longer required,
such as zalloc_dirent().

--

From the GNU libc manual [3]:
"
 It is expected that future versions of POSIX will obsolete readdir_r and
 mandate the level of thread safety for readdir which is provided by the
 GNU C Library and other implementations today.
"

There is a filed bug in the Austin Group Defect Tracker [4]  in which 'dalias'
proposes (in comment 0001632) that:
"
   I would like to propose an alternate solution. For readdir, replace the text:
    "The readdir() function need not be thread-safe."
   with:
    "If multiple threads call the readdir() function with the same directory
    stream argument and without synchronization to preclude simultaneous
    access, then the behavior is undefined."

   With this change, the clunky readdir_r function is no longer needed or
   useful, and should probably be deprecated. As the only reasonable way
   to meet the implementation requirements for readdir is to have the dirent
   buffer in the DIR structure, this change should not require any change to
   existing implementations.
"

[1] https://sourceware.org/ml/libc-alpha/2016-02/msg00093.html
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=19056
[3] https://www.gnu.org/software/libc/manual/html_node/Reading_002fClosing-Directory.html
[4] http://austingroupbugs.net/view.php?id=696

Signed-off-by: Chris Patterson <pattersonc@ainfosec.com>
---

v2:
- Additional detail in commit message
- Fix readdir loop logic (do not treat NULL as error)
- Cleanup additional (no longer used) code

v3:
- Restore original loop & error handling

 tools/libxl/libxl_pvusb.c | 24 ++++++------------------
 tools/libxl/libxl_utils.c |  8 +++-----
 2 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c
index 9f1e842..ec97ff8 100644
--- a/tools/libxl/libxl_pvusb.c
+++ b/tools/libxl/libxl_pvusb.c
@@ -508,19 +508,10 @@ int libxl_devid_to_device_usbctrl(libxl_ctx *ctx,
     return rc;
 }
 
-static void *zalloc_dirent(libxl__gc *gc, const char *dirpath)
-{
-    size_t need = offsetof(struct dirent, d_name) +
-                  pathconf(dirpath, _PC_NAME_MAX) + 1;
-
-    return libxl__zalloc(gc, need);
-}
-
 static char *usbdev_busaddr_to_busid(libxl__gc *gc, int bus, int addr)
 {
     DIR *dir;
     char *busid = NULL;
-    struct dirent *de_buf;
     struct dirent *de;
 
     /* invalid hostbus or hostaddr */
@@ -533,16 +524,15 @@ static char *usbdev_busaddr_to_busid(libxl__gc *gc, int bus, int addr)
         return NULL;
     }
 
-    de_buf = zalloc_dirent(gc, SYSFS_USB_DEV);
-
     for (;;) {
         char *filename;
         void *buf;
         int busnum = -1;
         int devnum = -1;
 
-        int r = readdir_r(dir, de_buf, &de);
-        if (r) {
+        errno = 0;
+        de = readdir(dir);
+        if (!de && errno) {
             LOGE(ERROR, "failed to readdir %s", SYSFS_USB_DEV);
             break;
         }
@@ -1157,7 +1147,6 @@ static int usbdev_get_all_interfaces(libxl__gc *gc, const char *busid,
 {
     DIR *dir;
     char *buf;
-    struct dirent *de_buf;
     struct dirent *de;
     int rc;
 
@@ -1172,12 +1161,11 @@ static int usbdev_get_all_interfaces(libxl__gc *gc, const char *busid,
         return ERROR_FAIL;
     }
 
-    de_buf = zalloc_dirent(gc, SYSFS_USB_DEV);
-
     for (;;) {
-        int r = readdir_r(dir, de_buf, &de);
+        errno = 0;
+        de = readdir(dir);
 
-        if (r) {
+        if (!de && errno) {
             LOGE(ERROR, "failed to readdir %s", SYSFS_USB_DEV);
             rc = ERROR_FAIL;
             goto out;
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index ceb8825..4ca6bcb 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -548,14 +548,12 @@ int libxl__remove_directory(libxl__gc *gc, const char *dirpath)
         goto out;
     }
 
-    size_t need = offsetof(struct dirent, d_name) +
-        pathconf(dirpath, _PC_NAME_MAX) + 1;
-    struct dirent *de_buf = libxl__zalloc(gc, need);
     struct dirent *de;
 
     for (;;) {
-        int r = readdir_r(d, de_buf, &de);
-        if (r) {
+        errno = 0;
+        de = readdir(d);
+        if (!de && errno) {
             LOGE(ERROR, "failed to readdir %s for removal", dirpath);
             rc = ERROR_FAIL;
             break;
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] libfsimage: replace deprecated readdir_r() with readdir()
  2016-06-03 16:50 [PATCH v3 1/2] libxl: replace deprecated readdir_r() with readdir() Chris Patterson
@ 2016-06-03 16:50 ` Chris Patterson
  2016-06-03 16:58   ` Ian Jackson
  2016-06-03 16:59 ` [PATCH v3 1/2] libxl: " Ian Jackson
  2017-02-22 12:04 ` George Dunlap
  2 siblings, 1 reply; 6+ messages in thread
From: Chris Patterson @ 2016-06-03 16:50 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson, Chris Patterson, wei.liu2

From: Chris Patterson <pattersonc@ainfosec.com>

Replace the usage of readdir_r() with readdir() to address a
compilation error under glibc due to the deprecation of readdir_r
for their next release (2.24) [1, 2].

Add new error checking on readdir(), and fail if error occurs.

--

From the GNU libc manual [3]:
"
 It is expected that future versions of POSIX will obsolete readdir_r and
 mandate the level of thread safety for readdir which is provided by the
 GNU C Library and other implementations today.
"

There is a filed bug in the Austin Group Defect Tracker [4]  in which 'dalias'
proposes (in comment 0001632) that:
"
   I would like to propose an alternate solution. For readdir, replace the text:
    "The readdir() function need not be thread-safe."
   with:
    "If multiple threads call the readdir() function with the same directory
    stream argument and without synchronization to preclude simultaneous
    access, then the behavior is undefined."

   With this change, the clunky readdir_r function is no longer needed or
   useful, and should probably be deprecated. As the only reasonable way
   to meet the implementation requirements for readdir is to have the dirent
   buffer in the DIR structure, this change should not require any change to
   existing implementations.
"

[1] https://sourceware.org/ml/libc-alpha/2016-02/msg00093.html
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=19056
[3] https://www.gnu.org/software/libc/manual/html_node/Reading_002fClosing-Directory.html
[4] http://austingroupbugs.net/view.php?id=696

Signed-off-by: Chris Patterson <pattersonc@ainfosec.com>
---

v2:
- Additional detail in commit message
- Cleanup additional (no longer used) code

v3:
- Add error check and fail if readdir() fails.

 tools/libfsimage/common/fsimage_plugin.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/tools/libfsimage/common/fsimage_plugin.c b/tools/libfsimage/common/fsimage_plugin.c
index 3fa06c7..0744e7b 100644
--- a/tools/libfsimage/common/fsimage_plugin.c
+++ b/tools/libfsimage/common/fsimage_plugin.c
@@ -123,7 +123,6 @@ static int load_plugins(void)
 {
 	const char *fsdir = getenv("FSIMAGE_FSDIR");
 	struct dirent *dp = NULL;
-	struct dirent *dpp;
 	DIR *dir = NULL;
 	char *tmp = NULL;
 	size_t name_max;
@@ -139,22 +138,26 @@ static int load_plugins(void)
 	if ((tmp = malloc(name_max + 1)) == NULL)
 		goto fail;
 
-	if ((dp = malloc(sizeof (struct dirent) + name_max + 1)) == NULL)
-		goto fail;
-
 	if ((dir = opendir(fsdir)) == NULL)
 		goto fail;
 
-	bzero(dp, sizeof (struct dirent) + name_max + 1);
+	for (;;) {
+		errno = 0;
+		dp = readdir(dir);
+
+		if (dp == NULL && errno != 0)
+			goto fail;
+
+		if (dp == NULL)
+			break;
 
-	while (readdir_r(dir, dp, &dpp) == 0 && dpp != NULL) {
-		if (strcmp(dpp->d_name, ".") == 0)
+		if (strcmp(dp->d_name, ".") == 0)
 			continue;
-		if (strcmp(dpp->d_name, "..") == 0)
+		if (strcmp(dp->d_name, "..") == 0)
 			continue;
 
 		(void) snprintf(tmp, name_max, "%s/%s/fsimage.so", fsdir,
-			dpp->d_name);
+			dp->d_name);
 
 		if (init_plugin(tmp) != 0)
 			goto fail;
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] libfsimage: replace deprecated readdir_r() with readdir()
  2016-06-03 16:50 ` [PATCH v3 2/2] libfsimage: " Chris Patterson
@ 2016-06-03 16:58   ` Ian Jackson
  0 siblings, 0 replies; 6+ messages in thread
From: Ian Jackson @ 2016-06-03 16:58 UTC (permalink / raw)
  To: Chris Patterson; +Cc: Chris Patterson, wei.liu2, xen-devel

Chris Patterson writes ("[PATCH v3 2/2] libfsimage: replace deprecated readdir_r() with readdir()"):
> From: Chris Patterson <pattersonc@ainfosec.com>
> 
> Replace the usage of readdir_r() with readdir() to address a
> compilation error under glibc due to the deprecation of readdir_r
> for their next release (2.24) [1, 2].
> 
> Add new error checking on readdir(), and fail if error occurs.

Thanks!

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] libxl: replace deprecated readdir_r() with readdir()
  2016-06-03 16:50 [PATCH v3 1/2] libxl: replace deprecated readdir_r() with readdir() Chris Patterson
  2016-06-03 16:50 ` [PATCH v3 2/2] libfsimage: " Chris Patterson
@ 2016-06-03 16:59 ` Ian Jackson
  2016-06-03 17:09   ` Wei Liu
  2017-02-22 12:04 ` George Dunlap
  2 siblings, 1 reply; 6+ messages in thread
From: Ian Jackson @ 2016-06-03 16:59 UTC (permalink / raw)
  To: Chris Patterson; +Cc: Chris Patterson, wei.liu2, xen-devel

Chris Patterson writes ("[PATCH v3 1/2] libxl: replace deprecated readdir_r() with readdir()"):
> From: Chris Patterson <pattersonc@ainfosec.com>
> 
> Replace the usage of readdir_r() with readdir() to address a
> compilation error under glibc due to the deprecation of readdir_r
> for their next release (2.24) [1, 2].
> 
> Remove code specific to usage of readdir_r which is no longer required,
> such as zalloc_dirent().

Great, thanks.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] libxl: replace deprecated readdir_r() with readdir()
  2016-06-03 16:59 ` [PATCH v3 1/2] libxl: " Ian Jackson
@ 2016-06-03 17:09   ` Wei Liu
  0 siblings, 0 replies; 6+ messages in thread
From: Wei Liu @ 2016-06-03 17:09 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Chris Patterson, Chris Patterson, wei.liu2, xen-devel

On Fri, Jun 03, 2016 at 05:59:42PM +0100, Ian Jackson wrote:
> Chris Patterson writes ("[PATCH v3 1/2] libxl: replace deprecated readdir_r() with readdir()"):
> > From: Chris Patterson <pattersonc@ainfosec.com>
> > 
> > Replace the usage of readdir_r() with readdir() to address a
> > compilation error under glibc due to the deprecation of readdir_r
> > for their next release (2.24) [1, 2].
> > 
> > Remove code specific to usage of readdir_r which is no longer required,
> > such as zalloc_dirent().
> 
> Great, thanks.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 

Thanks both.

I think we can take this series for 4.7.

I've queued it up.

Wei.

> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] libxl: replace deprecated readdir_r() with readdir()
  2016-06-03 16:50 [PATCH v3 1/2] libxl: replace deprecated readdir_r() with readdir() Chris Patterson
  2016-06-03 16:50 ` [PATCH v3 2/2] libfsimage: " Chris Patterson
  2016-06-03 16:59 ` [PATCH v3 1/2] libxl: " Ian Jackson
@ 2017-02-22 12:04 ` George Dunlap
  2 siblings, 0 replies; 6+ messages in thread
From: George Dunlap @ 2017-02-22 12:04 UTC (permalink / raw)
  To: Chris Patterson
  Cc: Wei Liu, Chris Patterson, Ian Jackson, Jan Beulich, xen-devel

On Fri, Jun 3, 2016 at 5:50 PM, Chris Patterson <cjp256@gmail.com> wrote:
> From: Chris Patterson <pattersonc@ainfosec.com>
>
> Replace the usage of readdir_r() with readdir() to address a
> compilation error under glibc due to the deprecation of readdir_r
> for their next release (2.24) [1, 2].
>
> Remove code specific to usage of readdir_r which is no longer required,
> such as zalloc_dirent().
>
> --
>
> From the GNU libc manual [3]:
> "
>  It is expected that future versions of POSIX will obsolete readdir_r and
>  mandate the level of thread safety for readdir which is provided by the
>  GNU C Library and other implementations today.
> "
>
> There is a filed bug in the Austin Group Defect Tracker [4]  in which 'dalias'
> proposes (in comment 0001632) that:
> "
>    I would like to propose an alternate solution. For readdir, replace the text:
>     "The readdir() function need not be thread-safe."
>    with:
>     "If multiple threads call the readdir() function with the same directory
>     stream argument and without synchronization to preclude simultaneous
>     access, then the behavior is undefined."
>
>    With this change, the clunky readdir_r function is no longer needed or
>    useful, and should probably be deprecated. As the only reasonable way
>    to meet the implementation requirements for readdir is to have the dirent
>    buffer in the DIR structure, this change should not require any change to
>    existing implementations.
> "
>
> [1] https://sourceware.org/ml/libc-alpha/2016-02/msg00093.html
> [2] https://sourceware.org/bugzilla/show_bug.cgi?id=19056
> [3] https://www.gnu.org/software/libc/manual/html_node/Reading_002fClosing-Directory.html
> [4] http://austingroupbugs.net/view.php?id=696
>
> Signed-off-by: Chris Patterson <pattersonc@ainfosec.com>

Jan,

These patches should be backported to the 4.6 branch (and earlier?) as well.

The commits are b9daff9 and c2a1786 if you want to cherry-pick them.
(b9daff9 doesn't apply quite cleanly because libxl_pvusb.c doesn't
exist).

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-02-22 12:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-03 16:50 [PATCH v3 1/2] libxl: replace deprecated readdir_r() with readdir() Chris Patterson
2016-06-03 16:50 ` [PATCH v3 2/2] libfsimage: " Chris Patterson
2016-06-03 16:58   ` Ian Jackson
2016-06-03 16:59 ` [PATCH v3 1/2] libxl: " Ian Jackson
2016-06-03 17:09   ` Wei Liu
2017-02-22 12:04 ` George Dunlap

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).