Re: [cifs:for-next 7/9] fs/cifs/file.c:3203:7-14: WARNING: Unsigned expression compared with zero: cur_len < 0 (fwd)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 1, 2018 at 10:18 AM Julia Lawall <julia.lawall@xxxxxxx> wrote:
>
> On Thu, 1 Nov 2018, Steve French wrote:
> >
> > ssize_t iov_iter_get_pages_alloc(struct iov_iter *i,
> >                    struct page ***pages, size_t maxsize,
> >                    size_t *start)
>
> The return type is ssize_t not size_t.  Putting a negative value into a
> size_t variable makes it positive


Aah - I misread the extra s (ssize_t vs. size_t) that is missing in his patch.

How about this trivial fix (attached)  -





> > {
> >         struct page **p;
> >
> >         if (maxsize > i->count)
> >                 maxsize = i->count;
> >
> >         if (unlikely(i->type & ITER_PIPE))
> >                 return pipe_get_pages_alloc(i, pages, maxsize, start);
> >         iterate_all_kinds(i, maxsize, v, ({
> >                 unsigned long addr = (unsigned long)v.iov_base;
> >                 size_t len = v.iov_len + (*start = addr & (PAGE_SIZE - 1));
> >                 int n;
> >                 int res;
> >
> >                 addr &= ~(PAGE_SIZE - 1);
> >                 n = DIV_ROUND_UP(len, PAGE_SIZE);
> >                 p = get_pages_array(n);
> >                 if (!p)
> >                         return -ENOMEM;
> >                 res = get_user_pages_fast(addr, n, (i->type & WRITE)
> > != WRITE, p);
> >                 if (unlikely(res < 0)) {
> >                         kvfree(p);
> >                         return res;
> >                 }
> >                 *pages = p;
> >                 return (res == n ? len : res * PAGE_SIZE) - *start;
> >         0;}),({
> >                 /* can't be more than PAGE_SIZE */
> >                 *start = v.bv_offset;
> >                 *pages = p = get_pages_array(1);
> >                 if (!p)
> >                         return -ENOMEM;
> >                 get_page(*p = v.bv_page);
> >                 return v.bv_len;
> >         }),({
> >                 return -EFAULT;
> >         })
> > On Thu, Nov 1, 2018 at 6:24 AM Julia Lawall <julia.lawall@xxxxxxx> wrote:
> > >
> > > Cur_len can't be less than 0 because it has type size_t, which is
> > > unsigned.
> > >
> > > julia
> > >
> > > ---------- Forwarded message ----------
> > > Date: Thu, 1 Nov 2018 11:37:21 +0800
> > > From: kbuild test robot <lkp@xxxxxxxxx>
> > > To: kbuild@xxxxxx
> > > Cc: Julia Lawall <julia.lawall@xxxxxxx>
> > > Subject: [cifs:for-next 7/9] fs/cifs/file.c:3203:7-14: WARNING: Unsigned
> > >     expression compared with zero: cur_len < 0
> > >
> > > CC: kbuild-all@xxxxxx
> > > CC: linux-cifs@xxxxxxxxxxxxxxx
> > > CC: samba-technical@xxxxxxxxxxxxxxx
> > > TO: Long Li <longli@xxxxxxxxxxxxx>
> > > CC: Steve French <stfrench@xxxxxxxxxxxxx>
> > >
> > > tree:   git://git.samba.org/sfrench/cifs-2.6.git for-next
> > > head:   7d8ceb129086fc6c0537fa37d6a4b65f490da006
> > > commit: 27d00133fadd8753e1856889dea2da4cb22910a3 [7/9] CIFS: Add support for direct I/O read
> > > :::::: branch date:
> > > :::::: commit date: 3 hours ago
> > >
> > > >> fs/cifs/file.c:3203:7-14: WARNING: Unsigned expression compared with zero: cur_len < 0
> > >
> > > git remote add cifs git://git.samba.org/sfrench/cifs-2.6.git
> > > git remote update cifs
> > > git checkout 27d00133fadd8753e1856889dea2da4cb22910a3
> > > vim +3203 fs/cifs/file.c
> > >
> > > 27d00133 Long Li          2018-10-31  3164
> > > d70b9104 Pavel Shilovsky  2016-11-17  3165  static int
> > > 0ada36b2 Pavel Shilovsky  2014-06-25  3166  cifs_send_async_read(loff_t offset, size_t len, struct cifsFileInfo *open_file,
> > > 6685c5e2 Pavel Shilovsky  2017-04-25  3167                   struct cifs_sb_info *cifs_sb, struct list_head *rdata_list,
> > > 6685c5e2 Pavel Shilovsky  2017-04-25  3168                   struct cifs_aio_ctx *ctx)
> > > ^1da177e Linus Torvalds   2005-04-16  3169  {
> > > 0ada36b2 Pavel Shilovsky  2014-06-25  3170      struct cifs_readdata *rdata;
> > > bed9da02 Pavel Shilovsky  2014-06-25  3171      unsigned int npages, rsize, credits;
> > > 0ada36b2 Pavel Shilovsky  2014-06-25  3172      size_t cur_len;
> > > 0ada36b2 Pavel Shilovsky  2014-06-25  3173      int rc;
> > > 1c892549 Jeff Layton      2012-05-16  3174      pid_t pid;
> > > 25f40259 Pavel Shilovsky  2014-06-25  3175      struct TCP_Server_Info *server;
> > > 27d00133 Long Li          2018-10-31  3176      struct page **pagevec;
> > > 27d00133 Long Li          2018-10-31  3177      size_t start;
> > > 27d00133 Long Li          2018-10-31  3178      struct iov_iter direct_iov = ctx->iter;
> > > a70307ee Pavel Shilovsky  2010-12-14  3179
> > > 25f40259 Pavel Shilovsky  2014-06-25  3180      server = tlink_tcon(open_file->tlink)->ses->server;
> > > fc9c5966 Pavel Shilovsky  2012-09-18  3181
> > > d4ffff1f Pavel Shilovsky  2011-05-26  3182      if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
> > > d4ffff1f Pavel Shilovsky  2011-05-26  3183              pid = open_file->pid;
> > > d4ffff1f Pavel Shilovsky  2011-05-26  3184      else
> > > d4ffff1f Pavel Shilovsky  2011-05-26  3185              pid = current->tgid;
> > > d4ffff1f Pavel Shilovsky  2011-05-26  3186
> > > 27d00133 Long Li          2018-10-31  3187      if (ctx->direct_io)
> > > 27d00133 Long Li          2018-10-31  3188              iov_iter_advance(&direct_iov, offset - ctx->pos);
> > > 27d00133 Long Li          2018-10-31  3189
> > > 1c892549 Jeff Layton      2012-05-16  3190      do {
> > > bed9da02 Pavel Shilovsky  2014-06-25  3191              rc = server->ops->wait_mtu_credits(server, cifs_sb->rsize,
> > > bed9da02 Pavel Shilovsky  2014-06-25  3192                                                 &rsize, &credits);
> > > bed9da02 Pavel Shilovsky  2014-06-25  3193              if (rc)
> > > bed9da02 Pavel Shilovsky  2014-06-25  3194                      break;
> > > bed9da02 Pavel Shilovsky  2014-06-25  3195
> > > bed9da02 Pavel Shilovsky  2014-06-25  3196              cur_len = min_t(const size_t, len, rsize);
> > > a70307ee Pavel Shilovsky  2010-12-14  3197
> > > 27d00133 Long Li          2018-10-31  3198              if (ctx->direct_io) {
> > > 27d00133 Long Li          2018-10-31  3199
> > > 27d00133 Long Li          2018-10-31  3200                      cur_len = iov_iter_get_pages_alloc(
> > > 27d00133 Long Li          2018-10-31  3201                                      &direct_iov, &pagevec,
> > > 27d00133 Long Li          2018-10-31  3202                                      cur_len, &start);
> > > 27d00133 Long Li          2018-10-31 @3203                      if (cur_len < 0) {
> > > 27d00133 Long Li          2018-10-31  3204                              cifs_dbg(VFS,
> > > 27d00133 Long Li          2018-10-31  3205                                      "couldn't get user pages (cur_len=%zd)"
> > > 27d00133 Long Li          2018-10-31  3206                                      " iter type %d"
> > > 27d00133 Long Li          2018-10-31  3207                                      " iov_offset %zd count %zd\n",
> > > 27d00133 Long Li          2018-10-31  3208                                      cur_len, direct_iov.type, direct_iov.iov_offset,
> > > 27d00133 Long Li          2018-10-31  3209                                      direct_iov.count);
> > > 27d00133 Long Li          2018-10-31  3210                              dump_stack();
> > > 27d00133 Long Li          2018-10-31  3211                              break;
> > > 27d00133 Long Li          2018-10-31  3212                      }
> > > 27d00133 Long Li          2018-10-31  3213                      iov_iter_advance(&direct_iov, cur_len);
> > > 27d00133 Long Li          2018-10-31  3214
> > > 27d00133 Long Li          2018-10-31  3215                      rdata = cifs_readdata_direct_alloc(
> > > 27d00133 Long Li          2018-10-31  3216                                      pagevec, cifs_uncached_readv_complete);
> > > 27d00133 Long Li          2018-10-31  3217                      if (!rdata) {
> > > 27d00133 Long Li          2018-10-31  3218                              add_credits_and_wake_if(server, credits, 0);
> > > 27d00133 Long Li          2018-10-31  3219                              rc = -ENOMEM;
> > > 27d00133 Long Li          2018-10-31  3220                              break;
> > > 27d00133 Long Li          2018-10-31  3221                      }
> > > 27d00133 Long Li          2018-10-31  3222
> > > 27d00133 Long Li          2018-10-31  3223                      npages = (cur_len + start + PAGE_SIZE-1) / PAGE_SIZE;
> > > 27d00133 Long Li          2018-10-31  3224                      rdata->page_offset = start;
> > > 27d00133 Long Li          2018-10-31  3225                      rdata->tailsz = npages > 1 ?
> > > 27d00133 Long Li          2018-10-31  3226                              cur_len-(PAGE_SIZE-start)-(npages-2)*PAGE_SIZE :
> > > 27d00133 Long Li          2018-10-31  3227                              cur_len;
> > > 27d00133 Long Li          2018-10-31  3228
> > > 27d00133 Long Li          2018-10-31  3229              } else {
> > > 27d00133 Long Li          2018-10-31  3230
> > > 27d00133 Long Li          2018-10-31  3231                      npages = DIV_ROUND_UP(cur_len, PAGE_SIZE);
> > > 1c892549 Jeff Layton      2012-05-16  3232                      /* allocate a readdata struct */
> > > 1c892549 Jeff Layton      2012-05-16  3233                      rdata = cifs_readdata_alloc(npages,
> > > 1c892549 Jeff Layton      2012-05-16  3234                                          cifs_uncached_readv_complete);
> > > 1c892549 Jeff Layton      2012-05-16  3235                      if (!rdata) {
> > > bed9da02 Pavel Shilovsky  2014-06-25  3236                              add_credits_and_wake_if(server, credits, 0);
> > > 1c892549 Jeff Layton      2012-05-16  3237                              rc = -ENOMEM;
> > > bae9f746 Jeff Layton      2014-04-15  3238                              break;
> > > ^1da177e Linus Torvalds   2005-04-16  3239                      }
> > > 1c892549 Jeff Layton      2012-05-16  3240
> > > c5fab6f4 Jeff Layton      2012-09-19  3241                      rc = cifs_read_allocate_pages(rdata, npages);
> > > 1c892549 Jeff Layton      2012-05-16  3242                      if (rc)
> > > 1c892549 Jeff Layton      2012-05-16  3243                              goto error;
> > > 1c892549 Jeff Layton      2012-05-16  3244
> > > 27d00133 Long Li          2018-10-31  3245                      rdata->tailsz = PAGE_SIZE;
> > > 27d00133 Long Li          2018-10-31  3246              }
> > > 27d00133 Long Li          2018-10-31  3247
> > > 1c892549 Jeff Layton      2012-05-16  3248              rdata->cfile = cifsFileInfo_get(open_file);
> > > c5fab6f4 Jeff Layton      2012-09-19  3249              rdata->nr_pages = npages;
> > > 1c892549 Jeff Layton      2012-05-16  3250              rdata->offset = offset;
> > > 1c892549 Jeff Layton      2012-05-16  3251              rdata->bytes = cur_len;
> > > 1c892549 Jeff Layton      2012-05-16  3252              rdata->pid = pid;
> > > 8321fec4 Jeff Layton      2012-09-19  3253              rdata->pagesz = PAGE_SIZE;
> > > 8321fec4 Jeff Layton      2012-09-19  3254              rdata->read_into_pages = cifs_uncached_read_into_pages;
> > > d70b9104 Pavel Shilovsky  2016-11-17  3255              rdata->copy_into_pages = cifs_uncached_copy_into_pages;
> > > bed9da02 Pavel Shilovsky  2014-06-25  3256              rdata->credits = credits;
> > > 6685c5e2 Pavel Shilovsky  2017-04-25  3257              rdata->ctx = ctx;
> > > 6685c5e2 Pavel Shilovsky  2017-04-25  3258              kref_get(&ctx->refcount);
> > > 1c892549 Jeff Layton      2012-05-16  3259
> > > 25f40259 Pavel Shilovsky  2014-06-25  3260              if (!rdata->cfile->invalidHandle ||
> > > 1fa839b4 Germano Percossi 2017-04-07  3261                  !(rc = cifs_reopen_file(rdata->cfile, true)))
> > > 25f40259 Pavel Shilovsky  2014-06-25  3262                      rc = server->ops->async_readv(rdata);
> > > 1c892549 Jeff Layton      2012-05-16  3263  error:
> > > 1c892549 Jeff Layton      2012-05-16  3264              if (rc) {
> > > bed9da02 Pavel Shilovsky  2014-06-25  3265                      add_credits_and_wake_if(server, rdata->credits, 0);
> > > 1c892549 Jeff Layton      2012-05-16  3266                      kref_put(&rdata->refcount,
> > > 1c892549 Jeff Layton      2012-05-16  3267                              cifs_uncached_readdata_release);
> > > 27d00133 Long Li          2018-10-31  3268                      if (rc == -EAGAIN) {
> > > 27d00133 Long Li          2018-10-31  3269                              iov_iter_revert(&direct_iov, cur_len);
> > > 25f40259 Pavel Shilovsky  2014-06-25  3270                              continue;
> > > 27d00133 Long Li          2018-10-31  3271                      }
> > > ^1da177e Linus Torvalds   2005-04-16  3272                      break;
> > > ^1da177e Linus Torvalds   2005-04-16  3273              }
> > > 1c892549 Jeff Layton      2012-05-16  3274
> > > 0ada36b2 Pavel Shilovsky  2014-06-25  3275              list_add_tail(&rdata->list, rdata_list);
> > > 1c892549 Jeff Layton      2012-05-16  3276              offset += cur_len;
> > > 1c892549 Jeff Layton      2012-05-16  3277              len -= cur_len;
> > > 1c892549 Jeff Layton      2012-05-16  3278      } while (len > 0);
> > > 1c892549 Jeff Layton      2012-05-16  3279
> > > 0ada36b2 Pavel Shilovsky  2014-06-25  3280      return rc;
> > > 0ada36b2 Pavel Shilovsky  2014-06-25  3281  }
> > > 0ada36b2 Pavel Shilovsky  2014-06-25  3282
> > >
> > > ---
> > > 0-DAY kernel test infrastructure                Open Source Technology Center
> > > https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve
> >



-- 
Thanks,

Steve
From 8ed3098c056a87b9e4b076c0f708807252deac06 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@xxxxxxxxxxxxx>
Date: Thu, 1 Nov 2018 10:54:32 -0500
Subject: [PATCH] cifs: fix signed/unsigned mismatch on aio_read patch

The patch "CIFS: Add support for direct I/O read" had
a signed/unsigned mismatch (ssize_t vs. size_t) in the
return from one function.

Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
Reported-by: Julia Lawall <julia.lawall@xxxxxxx>
---
 fs/cifs/file.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index fb3e0a594128..e8dd10bd7655 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2625,18 +2625,21 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
 			break;
 
 		if (ctx->direct_io) {
-			cur_len = iov_iter_get_pages_alloc(
+			ssize_t result;
+
+			result  = iov_iter_get_pages_alloc(
 				from, &pagevec, wsize, &start);
-			if (cur_len < 0) {
+			if (result < 0) {
 				cifs_dbg(VFS,
 					"direct_writev couldn't get user pages "
 					"(rc=%zd) iter type %d iov_offset %zd "
 					"count %zd\n",
-					cur_len, from->type,
+					result, from->type,
 					from->iov_offset, from->count);
 				dump_stack();
 				break;
 			}
+			cur_len = (size_t)result;
 			iov_iter_advance(from, cur_len);
 
 			nr_pages =
-- 
2.17.1


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux