[BUG] fs: cifs : does there exist a memleak in function cifs_writev_requeue

[BUG] fs: cifs : does there exist a memleak in function cifs_writev_requeue

[亿一]

Hi all:
 When reviewing the code of function cifs_writev_requeue, wdata2
allocated in while loop.
however,  if wdata2->cfile is NULL, the loop break without release
wdata2, there exists a memleak of wdata2?

static void
cifs_writev_requeue(struct cifs_writedata *wdata)
{
......
      wdata2 = cifs_writedata_alloc(nr_pages, cifs_writev_complete);
    // allocate wdata2
      if (!wdata2) {
             rc = -ENOMEM;
             break;
      }

      for (j = 0; j < nr_pages; j++) {
          wdata2->pages[j] = wdata->pages[i + j];
          lock_page(wdata2->pages[j]);
          clear_page_dirty_for_io(wdata2->pages[j]);
      }

      wdata2->sync_mode = wdata->sync_mode;
      wdata2->nr_pages = nr_pages;
      wdata2->offset = page_offset(wdata2->pages[0]);
      wdata2->pagesz = PAGE_SIZE;
      wdata2->tailsz = tailsz;
      wdata2->bytes = cur_len;

      wdata2->cfile = find_writable_file(CIFS_I(inode), false);
      if (!wdata2->cfile) {
            cifs_dbg(VFS, "No writable handles for inode\n");
            rc = -EBADF;
            break;                         // break without release wdata2.
      }
      ......
      }  while (i < wdata->nr_pages);

      mapping_set_error(inode->i_mapping, rc);
      kref_put(&wdata->refcount, cifs_writedata_release);
 }

Re: [BUG] fs: cifs : does there exist a memleak in function cifs_writev_requeue

[Aurélien Aptel]

Hi,

亿一 <teroincn@gmail.com> writes:
> Hi all:
>  When reviewing the code of function cifs_writev_requeue, wdata2
> allocated in while loop.
> however,  if wdata2->cfile is NULL, the loop break without release
> wdata2, there exists a memleak of wdata2?

Yes, good catch. It was fixed last year by the following commit:

commit 165df9a080b6
Author: Pavel Shilovsky <pshilov@microsoft.com>
Date:   Tue Jan 29 16:40:28 2019 -0800

    CIFS: Fix leaking locked VFS cache pages in writeback retry
    
    If we don't find a writable file handle when retrying writepages
    we break of the loop and do not unlock and put pages neither from
    wdata2 nor from the original wdata. Fix this by walking through
    all the remaining pages and cleanup them properly.

Cheers,
-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)