[Cluster-devel] qdisk - memcpy incorrect(?)

[Cluster-devel] qdisk - memcpy incorrect(?)

[Neale Ferguson]

Hi,
 In diskRawRead in disk.c there is the following code:

       readret = posix_memalign((void **)&alignedBuf, disk->d_pagesz, disk->d_blksz);
        if (readret < 0) {
                return -1;
        }

        io_state(STATE_READ);
        readret = read(disk->d_fd, alignedBuf, readlen);
        io_state(STATE_NONE);
        if (readret > 0) {
                if (readret > len) {
                        memcpy(alignedBuf, buf, len);
                        readret = len;
                } else {
                        memcpy(alignedBuf, buf, readret);
                }
        }

        free(alignedBuf);

The memcpy() above have the src/dst operands swapped. We read into alignedBuf and are supposed to copy to buf. I?m not sure why qdiskd works sometimes and not others.

--- cluster-3.0.12.1/cman/qdisk/disk.c    2013/05/16 16:45:49    1.1
+++ cluster-3.0.12.1/cman/qdisk/disk.c    2013/05/16 16:46:29
@@ -430,14 +430,14 @@
     io_state(STATE_READ);
     readret = read(disk->d_fd, alignedBuf, readlen);
     io_state(STATE_NONE);
     if (readret > 0) {
         if (readret > len) {
-            memcpy(alignedBuf, buf, len);
+            memcpy(buf, alignedBuf, len);
             readret = len;
         } else {
-            memcpy(alignedBuf, buf, readret);
+            memcpy(buf, alignedBuf, readret);
         }
     }

     free(alignedBuf);
     if (readret != len) {

Neale
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20130516/0b181dfb/attachment.htm>

[Cluster-devel] qdisk - memcpy incorrect(?)

[Fabio M. Di Nitto]

This is already fixed in more recent releases. See commit:

8edb0d0eb31d94b8a3ba81f6d5b4c398accc950d

your patch also misses another incorrect in diskRawWrite.

Fabio

On 05/16/2013 08:00 PM, Neale Ferguson wrote:
> Hi,
>  In diskRawRead in disk.c there is the following code:
> 
>        readret = posix_memalign((void **)&alignedBuf, disk->d_pagesz,
> disk->d_blksz);
>         if (readret < 0) {
>                 return -1;
>         }
> 
>         io_state(STATE_READ);
>         readret = read(disk->d_fd, alignedBuf, readlen);
>         io_state(STATE_NONE);
>         if (readret > 0) {
>                 if (readret > len) {
>                         memcpy(alignedBuf, buf, len);
>                         readret = len;
>                 } else {
>                         memcpy(alignedBuf, buf, readret);
>                 }
>         }
> 
>         free(alignedBuf);
> 
> The memcpy() above have the src/dst operands swapped. We read into
> alignedBuf and are supposed to copy to buf. I?m not sure why qdiskd
> works sometimes and not others.  
> 
> --- cluster-3.0.12.1/cman/qdisk/disk.c    2013/05/16 16:45:49    1.1
> +++ cluster-3.0.12.1/cman/qdisk/disk.c    2013/05/16 16:46:29
> @@ -430,14 +430,14 @@
>      io_state(STATE_READ);
>      readret = read(disk->d_fd, alignedBuf, readlen);
>      io_state(STATE_NONE);
>      if (readret > 0) {
>          if (readret > len) {
> -            memcpy(alignedBuf, buf, len);
> +            memcpy(buf, alignedBuf, len);
>              readret = len;
>          } else {
> -            memcpy(alignedBuf, buf, readret);
> +            memcpy(buf, alignedBuf, readret);
>          }
>      }
>  
>      free(alignedBuf);
>      if (readret != len) {
> 
> Neale