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

Bug#741952: linux: Possible bug in 3.2's cifs/file.c, use of uninitialized variable



Control: tag -1 patch

On Mon, 2014-03-17 at 16:51 +0100, Raphael Geissert wrote:
> Source: linux
> Version: 3.2.54-2
> 
> Hi,
> 
> In fs/cifs/file.c's cifs_iovec_write I believe that 'written'[1] can
> be used while not initialized: it is initialized in the call to
> CIFSSMBWrite2[2] but that code may not be run whenever
> cifs_reopen_file fails with any error other than EAGAIN. In that case,
> it would be used, uninitialized, to check it against 0[4] and then
> used to modify a series of size_t, ssize_t, loff_t, etc.
> 
> I have not tried to follow what could actually happen in that case.
> 
> >From a quick look to cifs_reopen_file it appears that at least EACCES
> and ENOMEM can be returned.
> 
> It would appear that this was fixed in 3.4 with the move to async
> writes in da82f7e755d2808ba726c9b23267d5bb23980e94
[...]

Agreed; what do you think of this patch?

Ben.

---
From: Ben Hutchings <ben@decadent.org.uk>
Date: Mon, 07 Apr 2014 03:29:24 +0100
Subject: cifs: cifs_iovec_write(): fix use of uninitialised var
Bug-Debian: https://bugs.debian.org/741952
Forwarded: not-needed

If the first call to cifs_reopen_file() from cifs_iovec_write() fails,
written is not initialised.  We must check rc before written.

---
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2194,15 +2194,17 @@ cifs_iovec_write(struct file *file, cons
 		for (i = 0; i < npages; i++)
 			kunmap(pages[i]);
 
+		if (rc < 0) {
+			if (!total_written)
+				total_written = rc;
+			break;
+		}
+
 		if (written) {
 			len -= written;
 			total_written += written;
 			cifs_update_eof(CIFS_I(inode), *poffset, written);
 			*poffset += written;
-		} else if (rc < 0) {
-			if (!total_written)
-				total_written = rc;
-			break;
 		}
 
 		/* get length and number of kvecs of the next write */


-- 
Ben Hutchings
Sturgeon's Law: Ninety percent of everything is crap.

Attachment: signature.asc
Description: This is a digitally signed message part


Reply to: