[Bug hurd/12187] New: [PATCH] sysdeps/mach/hurd: critical error values get overwritten

[Bug hurd/12187] New: [PATCH] sysdeps/mach/hurd: critical error values get overwritten

[nikai at nikai dot net]

http://sourceware.org/bugzilla/show_bug.cgi?id=12187

           Summary: [PATCH] sysdeps/mach/hurd: critical error values get
                    overwritten
           Product: glibc
           Version: 2.13
            Status: NEW
          Severity: normal
          Priority: P2
         Component: hurd
        AssignedTo: roland@gnu.org
        ReportedBy: nikai@nikai.net


Hi there!

I noticed three places where critical error values get
overwritten. It looks to me as if the intention might
instead have been to return the EIEIO value when the
'computer bought the farm'.

Best regards,
Nicolas Kaiser
---
 sysdeps/mach/hurd/bind.c    |    2 +-
 sysdeps/mach/hurd/connect.c |    2 +-
 sysdeps/mach/hurd/sendto.c  |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/sysdeps/mach/hurd/bind.c b/sysdeps/mach/hurd/bind.c
index a42b78a..996dc0b 100644
--- a/sysdeps/mach/hurd/bind.c
+++ b/sysdeps/mach/hurd/bind.c
@@ -101,7 +101,7 @@ __bind  (int fd, __CONST_SOCKADDR_ARG addrarg, socklen_t
len)
     return __hurd_fail (err);
     }
   else
-    err = EIEIO;
+    return __hurd_fail (EIEIO);

   err = HURD_DPORT_USE (fd,
             ({
diff --git a/sysdeps/mach/hurd/connect.c b/sysdeps/mach/hurd/connect.c
index 30883f6..cbba6c0 100644
--- a/sysdeps/mach/hurd/connect.c
+++ b/sysdeps/mach/hurd/connect.c
@@ -53,7 +53,7 @@ __connect (int fd, __CONST_SOCKADDR_ARG addrarg, socklen_t
len)
     return __hurd_fail (err);
     }
   else
-    err = EIEIO;
+    return __hurd_fail (EIEIO);

   err = HURD_DPORT_USE (fd,
             ({
diff --git a/sysdeps/mach/hurd/sendto.c b/sysdeps/mach/hurd/sendto.c
index 478a5c9..18b1106 100644
--- a/sysdeps/mach/hurd/sendto.c
+++ b/sysdeps/mach/hurd/sendto.c
@@ -54,7 +54,7 @@ __sendto (int fd,
     return __hurd_fail (err);
     }
   else
-    err = EIEIO;
+    return __hurd_fail (EIEIO);

   /* Get an address port for the desired destination address.  */
   err = HURD_DPORT_USE (fd,
-- 
1.7.2.2

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug hurd/12187] [PATCH] sysdeps/mach/hurd: critical error values get overwritten

[tschwinge at sourceware dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=12187

Thomas Schwinge <tschwinge at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING
                 CC|                            |tschwinge at sourceware dot
                   |                            |org
         AssignedTo|roland at gnu dot org       |tschwinge at sourceware dot
                   |                            |org

--- Comment #1 from Thomas Schwinge <tschwinge at sourceware dot org> 2012-02-18 09:21:08 UTC ---
Hi Nicolas!

Thanks for your report, and sorry for the delay in handling it.  My I ask
how/why you found this issue?

(The same pattern is also found in sendmsg.c.)

With the change you suggest, these calls would then only continue to work
for AF_LOCAL; don't you agree?  Here, that assignment to err is only to
initialize it to a non-zero value.  Then, in the HURD_DPORT_USE
construct's second argument, this is used to differentiate between the
AF_LOCAL case (where aport has already been bound), and all other AF_*
(where aport will now be bound by the socket_create_address call).  I
suspect that the value of EIEIO for initializing err has been chosen to
make it more obvious that it's a programming error, should this value
ever ``escape'' from the call.  Do you agree with this analyis?

I would happily accept a patch to make this construct easier to grasp.
>From a quick glance, we could probably do the following: initialize aport
to MACH_PORT_NULL and err to 0 at their definition sites, remote the err
assignment in else branch (not AF_LOCAL case), and instead use aport !=
MACH_PORT_NULL to conditionalize the socket_create_address call.  Would
that make it more obvious what is going on there?

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug hurd/12187] [PATCH] sysdeps/mach/hurd: critical error values get overwritten

[roland at gnu dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=12187

Roland McGrath <roland at gnu dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |RESOLVED
                 CC|                            |roland at gnu dot org
         Resolution|                            |INVALID

--- Comment #2 from Roland McGrath <roland at gnu dot org> 2012-02-21 19:47:52 UTC ---
Your analysis is correct.  There is no bug here.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug hurd/12187] [PATCH] sysdeps/mach/hurd: critical error values get overwritten

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=12187

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.