Bug 10371 - On big-endian machines getsockopt returns 0 (via optval) for optlen==1 when returned value should be 255
Summary: On big-endian machines getsockopt returns 0 (via optval) for optlen==1 when r...
Status: CLOSED CODE_FIX
Alias: None
Product: Networking
Classification: Unclassified
Component: IPV4 (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Stephen Hemminger
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-31 08:15 UTC by Marek Piechaczek
Modified: 2008-04-11 14:46 UTC (History)
1 user (show)

See Also:
Kernel Version: 2.6.24
Subsystem:
Regression: ---
Bisected commit-id:


Attachments
patch for net/ipv4/ip_sockglue.c (384 bytes, patch)
2008-04-01 01:34 UTC, Marek Piechaczek
Details | Diff

Description Marek Piechaczek 2008-03-31 08:15:39 UTC
Latest working kernel version: 
Earliest failing kernel version: all 2.6
Distribution: 
Hardware Environment: 
Software Environment: 
Problem Description: 

On big-endian machines getsockopt fails for optlen==1 when returned value is 255.
Eg. after calling
  unsigned char ttl = 255;
  socklen_t     len = sizeof(ttl);
  setsockopt(socket, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, &len);
the following call will fail:
  getsockopt(socket, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, &len);
"ttl" returned will be 0.


It's because of bug in /net/ipv4/ip_sockglue.c:

 static int do_ip_getsockopt(struct sock *sk, int level, int optname,
			    char __user *optval, int __user *optlen) 
 ....
	if (len < sizeof(int) && len > 0 && val>=0 && val<255) {
		unsigned char ucval = (unsigned char)val;
		len = 1;
		if (put_user(len, optlen))
			return -EFAULT;
		if (copy_to_user(optval,&ucval,1))
			return -EFAULT;
	} else {
		len = min_t(unsigned int, sizeof(int), len);
		if (put_user(len, optlen))
			return -EFAULT;
		if (copy_to_user(optval,&val,len))
			return -EFAULT;
	}
	return 0;
} 

The comparision in if-statement should be:
	if (len < sizeof(int) && len > 0 && val>=0 && val<=255)

Otherwise on big-endian machines copy_to_user(optval,&val,len) routine copies first (highest) byte which is 0 in that case.


This bug does not occur on low-endian machines since copy_to_user(optval,&val,len) copies right (lowest) byte in that case.
Comment 1 Marek Piechaczek 2008-03-31 08:22:47 UTC
Try to execute the following code on low- and big-endian machines:

#include <string.h>
#include <stdio.h>
#include <sys/socket.h>
#include <sys/ioctl.h>
# include <netdb.h>
# include <sys/socket.h>
# include <sys/ioctl.h>
# include <unistd.h>
# include <sys/select.h>
# include <netinet/tcp.h>

int main(void)
{
  const unsigned char TTL_INT   = 255;
  int rv;
  int s;
  int           ttl_int_val   = 0;
  unsigned char ttl_uchar_val = 0;
  socklen_t ttl_int_len   = sizeof(ttl_int_val);
  socklen_t ttl_uchar_len = sizeof(ttl_uchar_val);


  s = socket(AF_INET, SOCK_DGRAM, 0);
  printf ("s=%d\n", s);

  rv = setsockopt(s, IPPROTO_IP, IP_MULTICAST_TTL,
                 (void *)&TTL_INT, sizeof(TTL_INT));

  printf("rv=%d\n", rv);

  rv = getsockopt(s, IPPROTO_IP, IP_MULTICAST_TTL,
                 (void *)&ttl_int_val, &ttl_int_len);

  printf("rv=%d, ttl=%d len=%d\n", rv, ttl_int_val, ttl_int_len);

  rv = getsockopt(s, IPPROTO_IP, IP_MULTICAST_TTL,
                 (void *)&ttl_uchar_val, &ttl_uchar_len);

  printf("rv=%d, ttl=%d len=%d\n", rv, ttl_uchar_val, ttl_uchar_len);
}


On low-endian CPU the output is:
 s=4
 rv=0
 rv=0, ttl=255 len=4
 rv=0, ttl=255 len=1

On big-endian CPU the output is (incorrect):
 s=4
 rv=0
 rv=0, ttl=255 len=4
 rv=0, ttl=0 len=1
Comment 2 Anonymous Emailer 2008-03-31 11:33:54 UTC
Reply-To: akpm@linux-foundation.org

On Mon, 31 Mar 2008 08:16:05 -0700 (PDT) bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=10371
> 
>            Summary: On big-endian machines getsockopt returns 0 (via optval)
>                     for optlen==1 when returned value should 255
>            Product: Networking
>            Version: 2.5
>      KernelVersion: 2.6.24
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: IPV4
>         AssignedTo: shemminger@linux-foundation.org
>         ReportedBy: M.Piechaczek@osmosys.tv
> 
> 
> Latest working kernel version: 
> Earliest failing kernel version: all 2.6
> Distribution: 
> Hardware Environment: 
> Software Environment: 
> Problem Description: 
> 
> On big-endian machines getsockopt fails for optlen==1 when returned value is
> 255.
> Eg. after calling
>   unsigned char ttl = 255;
>   socklen_t     len = sizeof(ttl);
>   setsockopt(socket, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, &len);
> the following call will fail:
>   getsockopt(socket, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, &len);
> "ttl" returned will be 0.
> 
> 
> It's because of bug in /net/ipv4/ip_sockglue.c:
> 
>  static int do_ip_getsockopt(struct sock *sk, int level, int optname,
>                             char __user *optval, int __user *optlen) 
>  ....
>         if (len < sizeof(int) && len > 0 && val>=0 && val<255) {
>                 unsigned char ucval = (unsigned char)val;
>                 len = 1;
>                 if (put_user(len, optlen))
>                         return -EFAULT;
>                 if (copy_to_user(optval,&ucval,1))
>                         return -EFAULT;
>         } else {
>                 len = min_t(unsigned int, sizeof(int), len);
>                 if (put_user(len, optlen))
>                         return -EFAULT;
>                 if (copy_to_user(optval,&val,len))
>                         return -EFAULT;
>         }
>         return 0;
> } 
> 
> The comparision in if-statement should be:
>         if (len < sizeof(int) && len > 0 && val>=0 && val<=255)
> 
> Otherwise on big-endian machines copy_to_user(optval,&val,len) routine copies
> first (highest) byte which is 0 in that case.
> 
> 
> This bug does not occur on low-endian machines since
> copy_to_user(optval,&val,len) copies right (lowest) byte in that case.
> 

You've done all the hard work - please email us a tested patch?
Comment 3 Marek Piechaczek 2008-04-01 01:34:03 UTC
Created attachment 15549 [details]
patch for net/ipv4/ip_sockglue.c
Comment 4 David S. Miller 2008-04-10 01:32:30 UTC
From: Andrew Morton <akpm@linux-foundation.org>
Date: Mon, 31 Mar 2008 11:33:49 -0700

> On Mon, 31 Mar 2008 08:16:05 -0700 (PDT) bugme-daemon@bugzilla.kernel.org
> wrote:
> 
> > http://bugzilla.kernel.org/show_bug.cgi?id=10371
 ...
> You've done all the hard work - please email us a tested patch?

I've checked in the following fix for this bug, thanks!

commit 951e07c930f5f66b676eaa4c32a1b0d8e2d7d06a
Author: David S. Miller <davem@davemloft.net>
Date:   Thu Apr 10 01:29:36 2008 -0700

    [IPV4]: Fix byte value boundary check in do_ip_getsockopt().
    
    This fixes kernel bugzilla 10371.
    
    As reported by M.Piechaczek@osmosys.tv, if we try to grab a
    char sized socket option value, as in:
    
      unsigned char ttl = 255;
      socklen_t     len = sizeof(ttl);
      setsockopt(socket, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, &len);
    
      getsockopt(socket, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, &len);
    
    The ttl returned will be wrong on big-endian, and on both little-
    endian and big-endian the next three bytes in userspace are written
    with garbage.
    
    It's because of this test in do_ip_getsockopt():
    
    	if (len < sizeof(int) && len > 0 && val>=0 && val<255) {
    
    It should allow a 'val' of 255 to pass here, but it doesn't so it
    copies a full 'int' back to userspace.
    
    On little-endian that will write the correct value into the location
    but it spams on the next three bytes in userspace.  On big endian it
    writes the wrong value into the location and spams the next three
    bytes.
    
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index f72457b..c2921d0 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1132,7 +1132,7 @@ static int do_ip_getsockopt(struct sock *sk, int level, int optname,
 	}
 	release_sock(sk);
 
-	if (len < sizeof(int) && len > 0 && val>=0 && val<255) {
+	if (len < sizeof(int) && len > 0 && val>=0 && val<=255) {
 		unsigned char ucval = (unsigned char)val;
 		len = 1;
 		if (put_user(len, optlen))
Comment 5 Adrian Bunk 2008-04-11 14:46:01 UTC
fixed by commit 951e07c930f5f66b676eaa4c32a1b0d8e2d7d06a

Note You need to log in before you can comment on or make changes to this bug.