Re: [stda@enea.se] libc/1623: buggs in XDR functions

[Sten Dahlgren <stda at enea dot se>]

Hello !

The sad thing is that i found that the same bugg is in xdr_hyper
ie the sign extension problem.

I'll try to fix this in the original source when at home and send you a 
proper diff on monday

sorry 

regards
/Sten

Andreas Jaeger wrote:
> 
> Hi glibc folks,
> 
> we've received the appended bug report which includes a fix [1].  I've
> checked this and do think it's correct.  I'm appending a proper diff.
> 
> Ulrich, please apply this.
> 
> Thanks,
> Andreas
> 
> Footnotes:
> [1] Sten, please send diff -u output next time, this shows better what
>     you do.
> 
> 2000-03-03  Andreas Jaeger  <aj@suse.de>
> 
>         * sunrpc/xdr_intXX_t.c (xdr_uint16_t): Fix ENCODE and DECODE
>         cases.
>         (xdr_int64_t): Correctly handle lower half.
>         Fix by Sten Dahlgren <stda@enea.se>, closes PR libc/1623.
> 
> ============================================================
> Index: sunrpc/xdr_intXX_t.c
> --- sunrpc/xdr_intXX_t.c        1999/04/08 02:09:57     1.2
> +++ sunrpc/xdr_intXX_t.c        2000/03/03 15:33:14
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 1998, 1999 Free Software Foundation, Inc.
> +/* Copyright (c) 1998, 1999, 2000 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>     Contributed by Thorsten Kukuk <kukuk@vt.uni-paderborn.de>, 1998.
> 
> @@ -25,7 +25,9 @@
>  xdr_int64_t (XDR *xdrs, int64_t *ip)
>  {
>    int32_t t1;
> -  int32_t t2;
> +  /* This must be unsigned, otherwise we get problems with sign
> +     extension in the DECODE case.  */
> +  uint32_t t2;
> 
>    switch (xdrs->x_op)
>      {
> @@ -139,11 +141,11 @@
> 
>    switch (xdrs->x_op)
>      {
> -    case XDR_DECODE:
> -      ut = (uint32_t) *uip;
> -      return XDR_GETINT32 (xdrs, (int32_t *) &ut);
>      case XDR_ENCODE:
> -      if (!XDR_PUTINT32 (xdrs, (int32_t *) &ut))
> +      ut = (uint32_t) *uip;
> +      return XDR_PUTINT32 (xdrs, (int32_t *) &ut);
> +    case XDR_DECODE:
> +      if (!XDR_GETINT32 (xdrs, (int32_t *) &ut))
>         return FALSE;
>        *uip = (uint16_t) ut;
>        return TRUE;
> 
>   ------------------------------------------------------------------------
> 
> Subject: libc/1623: buggs in XDR functions
> Date: Fri Mar 3 16:30:51 2000
> From: stda@enea.se
> 
> Topics:
>    libc/1623: buggs in XDR functions
> 
> ----------------------------------------------------------------------
> 
> Date: Thu, 2 Mar 2000 14:54:16 +0100
> From: stda@enea.se
> To: bugs@gnu.org
> Subject: libc/1623: buggs in XDR functions
> Message-Id: <200003021354.OAA02531@rtos002.enea.se>
> 
> >Number:         1623
> >Category:       libc
> >Synopsis:       buggs in XDR functions
> >Confidential:   no
> >Severity:       serious
> >Priority:       medium
> >Responsible:    libc-gnats
> >State:          open
> >Class:          sw-bug
> >Submitter-Id:   unknown
> >Arrival-Date:   Thu Mar 02 09:00:01 EST 2000
> >Last-Modified:
> >Originator:     Sten Dahlgren
> >Organization:
> 
> >Release:        libc-2.1.2
> >Environment:
> 
> Host type: i386-suse-linux-gnu
> System: Linux rtos002 2.2.13 #4 Tue Jan 18 13:39:10 CET 2000 i686 unknown
> Architecture: i686
> 
> Addons: crypt linuxthreads noversion nss-v1
> 
> Build CC: gcc
> Compiler version: egcs-2.91.66 19990314/Linux (egcs-1.1.2 release)
> Kernel headers: 2.2.13
> Symbol versioning: yes
> Build static: yes
> Build shared: yes
> Build pic-default: no
> Build profile: yes
> Build omitfp: no
> Build bounded: no
> Build static-nss: no
> Stdio: libio
> 
> >Description:
>         Bug in the xdr_uint16_t and xdr_int64_t code
> 
> >How-To-Repeat:
> 
> >Fix:
> 
> The original xdr_uint16_t was severely mangled. This code corrects the problem
> 
> 
> /* XDR 16bit unsigned integers */
> bool_t
> xdr_uint16_t (XDR *xdrs, uint16_t *uip)
> {
>   uint32_t ut;
> 
>   switch (xdrs->x_op)
>     {
>     case XDR_ENCODE:
>       ut = (uint32_t) *uip;
>       return XDR_PUTINT32 (xdrs, (int32_t *) &ut);
>     case XDR_DECODE:
>       if (!XDR_GETINT32 (xdrs, (int32_t *) &ut))
>         return FALSE;
>       *uip = (uint16_t) ut;
>       return TRUE;
>     case XDR_FREE:
>       return TRUE;
>     default:
>       return FALSE;
>     }
> }
> 
> The original had an error when doing "*ip |= t2". If t2 had a
> negative value a signed extension was made resulting in the upper
> part of *ip always was ones.
> 
> /* XDR 64bit integers */
> bool_t
> xdr_int64_t (XDR *xdrs, int64_t *ip)
> {
>   int32_t t1;
>   /* int32_t t2; buggy */
>   u_int32_t t2;
> 
>   switch (xdrs->x_op)
>     {
>     case XDR_ENCODE:
>       t1 = (int32_t) ((*ip) >> 32);
>       t2 = (int32_t) (*ip);
>       return (XDR_PUTINT32(xdrs, &t1) && XDR_PUTINT32(xdrs, &t2));
>     case XDR_DECODE:
>       if (!XDR_GETINT32(xdrs, &t1) || !XDR_GETINT32(xdrs, &t2))
>         return FALSE;
>       *ip = ((int64_t) t1) << 32;
>       *ip |= t2;
>       return TRUE;
>     case XDR_FREE:
>       return TRUE;
>     default:
>       return FALSE;
>     }
> }
> 
> regards
> 
> /Sten
> >Audit-Trail:
> >Unformatted:
> 
> ------------------------------
> 
> End of forwarddoHCgX Digest
> ***************************
> 
>   ------------------------------------------------------------------------
> 
> --
>  Andreas Jaeger
>   SuSE Labs aj@suse.de
>    private aj@arthur.rhein-neckar.de