[Linux-ia64] [Fwd: memcpy failure]

[Linux-ia64] [Fwd: memcpy failure]

[Christian Cotte-Barrot]

[-- Attachment #1: Type: text/plain, Size: 55 bytes --]

Forwarded this here because it looks like really a bug.

[-- Attachment #2: Type: message/rfc822, Size: 5576 bytes --]

From: Christian Cotte-Barrot <Christian.Cotte-Barrot@bull.net>
To: Ken Chen <kenneth.w.chen@intel.com>
Cc: David Mosberger <davidm@napali.hpl.hp.com>, Stephane Eranian <eranian@frankl.hpl.hp.com>, Cotte-Barrot Christian <Christian.Cotte-Barrot@bull.net>
Subject: memcpy failure
Date: Tue, 12 Nov 2002 16:51:19 +0100
Message-ID: <3DD12376.AC558193@bull.net>

Hi Ken,

sorry to disturb you directly, but I found your name in the source file
I have some concern with (./arch/ia64/lib/memcpy_mck.S)
I am currently porting a serial line driver to linux IA64 release 2.4.19
on a tiger machine.
This driver checks the return from the memcpy function that should be
a pointer to dest as the man says.
The return from the memcpy function seems to be always null.
I put some printk traces in the driver and it appeared that although
the return does not comply with the man the copy is correctly (hopefully) done
(the printk reports the first three and the last three bytes of the dest
area before and after the memcpy call and we can see that they have been
overwritten).
Is it right to check the return from memcpy ?

Here are some typical outputs picked up in /var/log/messages:
...
Nov  8 16:13:51 bza kernel: epca: receive_data : dest before memcpy  0x72 0x77 0x2D ...
0x6B 0x20 0x20
Nov  8 16:13:51 bza kernel: epca: receive_data : memcpy failed, dest 0xE000000071112468
src 0xC0000000E2C1A9DC n 512 rdest 0x0000000000000000
Nov  8 16:13:51 bza kernel: epca: receive_data : dest after memcpy  0x33 0x33 0x3B ...
0x20 0x20 0x20
...

From memcpy_mck.S, both memcpy and __copy_user functions return
the same null value initialized in the common_code part:
  ./arch/ia64/lib/memcpy_mck.S
    ...
  GLOBAL_ENTRY(memcpy)
          and     r28=0x7,in0
          and     r29=0x7,in1
          mov     f6=f0
          br.cond.sptk .common_code
          ;;
  END(memcpy)
  GLOBAL_ENTRY(__copy_user)
          .prologue
  // check dest alignment
          and     r28=0x7,in0
          and     r29=0x7,in1
          mov     f6=f1
          mov     saved_in0=in0   // save dest pointer
          mov     saved_in1=in1   // save src pointer
          mov     saved_in2=in2   // save len
          ;;
  .common_code:
          cmp.gt  p15,p0=8,in2    // check for small size
          cmp.ne  p13,p0=0,r28    // check dest alignment
          cmp.ne  p14,p0=0,r29    // check src alignment
          add     src0=0,in1
          sub     r30=8,r28       // for .align_dest
          mov     retval=r0       // initialize return value
          ;;
    ...

But According to the old sources memcpy.S and copy_user.S they
used to return different values, dest for memcpy and null for copy_user
  ./arch/ia64/lib/memcpy.S
    ...
    GLOBAL_ENTRY(memcpy)
    ...
          .body
          cmp.eq p6,p0=in2,r0     // zero length?
          mov retval=in0          // return dst
    ...

  ./arch/ia64/lib/copy_user.S
    ...
    GLOBAL_ENTRY(__copy_user)
    ...
          adds len2=-1,len        // br.ctop is repeat/until
          mov ret0=r0
    ...

It's certainly not the better way to modify memcpy_mck.S but
here is a simple and trivial patch I tried and that corrects
the problem:
  ./arch/ia64/lib/memcpy_mck.S
    ...
  GLOBAL_ENTRY(memcpy)
          and     r28=0x7,in0
          and     r29=0x7,in1
          mov     f6=f0
          mov retval=in0          // return dst
          br.cond.sptk .common_code
          ;;
  END(memcpy)
  GLOBAL_ENTRY(__copy_user)
          .prologue
  // check dest alignment
          and     r28=0x7,in0
          and     r29=0x7,in1
          mov     f6=f1
          mov     saved_in0=in0   // save dest pointer
          mov     saved_in1=in1   // save src pointer
          mov     saved_in2=in2   // save len
          mov     retval=r0       // initialize return value
          ;;
  .common_code:
          cmp.gt  p15,p0=8,in2    // check for small size
          cmp.ne  p13,p0=0,r28    // check dest alignment
          cmp.ne  p14,p0=0,r29    // check src alignment
          add     src0=0,in1
          sub     r30=8,r28       // for .align_dest
          ;;
    ...

Regards.
-- 
+===========+=======================+==================================+
|  |\/\/\/| |                       |                                  |
|  |      | |Christian Cotte-Barrot |org.  :BULL/                      |
|  | (~)(o) |Bull S.A.              |office:FREC/B1-401                |
| C      _) |1, rue de Provence     |mailto:                           |
|  | ,___|  |B.P. 208               |   Christian.Cotte-Barrot@bull.net|
|  |   /    |38432 ECHIROLLES CEDEX |phone :+33 (0)476297725 (229 7725)|
| /----\    |FRANCE                 |fax   :+33 (0)476297891 (229 7891)|
+===========+=======================+==================================+

[Linux-ia64] [Fwd: memcpy failure]

[Christian Cotte-Barrot]

[-- Attachment #1: Type: text/plain, Size: 6 bytes --]

ditto.

[-- Attachment #2: Type: message/rfc822, Size: 7236 bytes --]

From: "Chen, Kenneth W" <kenneth.w.chen@intel.com>
To: "Christian Cotte-Barrot" <Christian.Cotte-Barrot@bull.net>
Cc: "David Mosberger" <davidm@napali.hpl.hp.com>, "Stephane Eranian" <eranian@frankl.hpl.hp.com>
Subject: RE: memcpy failure
Date: Tue, 12 Nov 2002 10:29:23 -0800
Message-ID: <51568623CC066847A1DC7EB0D95B587A181FC0@fmsmsx405.fm.intel.com>

The retrun value for memcpy doesn't follow the user space memcpy exactly.
kernel memcpy always return 0.
- Ken

-----Original Message-----
From: Christian Cotte-Barrot [mailto:Christian.Cotte-Barrot@bull.net]
Sent: Tuesday, November 12, 2002 7:51 AM
To: Chen, Kenneth W
Cc: David Mosberger; Stephane Eranian; Cotte-Barrot Christian
Subject: memcpy failure


Hi Ken,

sorry to disturb you directly, but I found your name in the source file
I have some concern with (./arch/ia64/lib/memcpy_mck.S)
I am currently porting a serial line driver to linux IA64 release 2.4.19
on a tiger machine.
This driver checks the return from the memcpy function that should be
a pointer to dest as the man says.
The return from the memcpy function seems to be always null.
I put some printk traces in the driver and it appeared that although
the return does not comply with the man the copy is correctly (hopefully) done
(the printk reports the first three and the last three bytes of the dest
area before and after the memcpy call and we can see that they have been
overwritten).
Is it right to check the return from memcpy ?

Here are some typical outputs picked up in /var/log/messages:
...
Nov  8 16:13:51 bza kernel: epca: receive_data : dest before memcpy  0x72 0x77 0x2D ...
0x6B 0x20 0x20
Nov  8 16:13:51 bza kernel: epca: receive_data : memcpy failed, dest 0xE000000071112468
src 0xC0000000E2C1A9DC n 512 rdest 0x0000000000000000
Nov  8 16:13:51 bza kernel: epca: receive_data : dest after memcpy  0x33 0x33 0x3B ...
0x20 0x20 0x20
...

From memcpy_mck.S, both memcpy and __copy_user functions return
the same null value initialized in the common_code part:
  ./arch/ia64/lib/memcpy_mck.S
    ...
  GLOBAL_ENTRY(memcpy)
          and     r28=0x7,in0
          and     r29=0x7,in1
          mov     f6=f0
          br.cond.sptk .common_code
          ;;
  END(memcpy)
  GLOBAL_ENTRY(__copy_user)
          .prologue
  // check dest alignment
          and     r28=0x7,in0
          and     r29=0x7,in1
          mov     f6=f1
          mov     saved_in0=in0   // save dest pointer
          mov     saved_in1=in1   // save src pointer
          mov     saved_in2=in2   // save len
          ;;
  .common_code:
          cmp.gt  p15,p0=8,in2    // check for small size
          cmp.ne  p13,p0=0,r28    // check dest alignment
          cmp.ne  p14,p0=0,r29    // check src alignment
          add     src0=0,in1
          sub     r30=8,r28       // for .align_dest
          mov     retval=r0       // initialize return value
          ;;
    ...

But According to the old sources memcpy.S and copy_user.S they
used to return different values, dest for memcpy and null for copy_user
  ./arch/ia64/lib/memcpy.S
    ...
    GLOBAL_ENTRY(memcpy)
    ...
          .body
          cmp.eq p6,p0=in2,r0     // zero length?
          mov retval=in0          // return dst
    ...

  ./arch/ia64/lib/copy_user.S
    ...
    GLOBAL_ENTRY(__copy_user)
    ...
          adds len2=-1,len        // br.ctop is repeat/until
          mov ret0=r0
    ...

It's certainly not the better way to modify memcpy_mck.S but
here is a simple and trivial patch I tried and that corrects
the problem:
  ./arch/ia64/lib/memcpy_mck.S
    ...
  GLOBAL_ENTRY(memcpy)
          and     r28=0x7,in0
          and     r29=0x7,in1
          mov     f6=f0
          mov retval=in0          // return dst
          br.cond.sptk .common_code
          ;;
  END(memcpy)
  GLOBAL_ENTRY(__copy_user)
          .prologue
  // check dest alignment
          and     r28=0x7,in0
          and     r29=0x7,in1
          mov     f6=f1
          mov     saved_in0=in0   // save dest pointer
          mov     saved_in1=in1   // save src pointer
          mov     saved_in2=in2   // save len
          mov     retval=r0       // initialize return value
          ;;
  .common_code:
          cmp.gt  p15,p0=8,in2    // check for small size
          cmp.ne  p13,p0=0,r28    // check dest alignment
          cmp.ne  p14,p0=0,r29    // check src alignment
          add     src0=0,in1
          sub     r30=8,r28       // for .align_dest
          ;;
    ...

Regards.
-- 
+===========+=======================+==================================+
|  |\/\/\/| |                       |                                  |
|  |      | |Christian Cotte-Barrot |org.  :BULL/                      |
|  | (~)(o) |Bull S.A.              |office:FREC/B1-401                |
| C      _) |1, rue de Provence     |mailto:                           |
|  | ,___|  |B.P. 208               |   Christian.Cotte-Barrot@bull.net|
|  |   /    |38432 ECHIROLLES CEDEX |phone :+33 (0)476297725 (229 7725)|
| /----\    |FRANCE                 |fax   :+33 (0)476297891 (229 7891)|
+===========+=======================+==================================+

[Linux-ia64] [Fwd: memcpy failure]

[Christian Cotte-Barrot]

[-- Attachment #1: Type: text/plain, Size: 6 bytes --]

ditto.

[-- Attachment #2: Type: message/rfc822, Size: 2749 bytes --]

From: David Mosberger <davidm@napali.hpl.hp.com>
To: "Chen, Kenneth W" <kenneth.w.chen@intel.com>
Cc: "Christian Cotte-Barrot" <Christian.Cotte-Barrot@bull.net>, "David Mosberger" <davidm@napali.hpl.hp.com>, "Stephane Eranian" <eranian@frankl.hpl.hp.com>
Subject: RE: memcpy failure
Date: Wed, 13 Nov 2002 21:52:12 -0800
Message-ID: <15827.14860.687278.896618@napali.hpl.hp.com>

>>>>> On Tue, 12 Nov 2002 10:29:23 -0800, "Chen, Kenneth W" <kenneth.w.chen@intel.com> said:

  Ken> The retrun value for memcpy doesn't follow the user space
  Ken> memcpy exactly.  kernel memcpy always return 0.

Why are you saying this?  As far as I know, the kernel memcpy() is
expected to return the (initial) destination address.  Perhaps you're
thinking of copy_user()?

(If it was up to me, memcpy() would be returning void...).

	--david