public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Edward Shishkin <edward@namesys.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	"Vladimir V. Saveliev" <vs@clusterfs.com>
Cc: Dave Hansen <haveblue@us.ibm.com>, Zan Lynx <zlynx@acm.org>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	reiserfs-devel <reiserfs-devel@vger.kernel.org>,
	hch <hch@infradead.org>
Subject: [patch] reiser4: do not allocate struct file on stack
Date: Fri, 05 Oct 2007 03:22:22 +0400	[thread overview]
Message-ID: <470575AE.9060902@namesys.com> (raw)
In-Reply-To: <46FD8298.5020804@namesys.com>

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

Edward Shishkin wrote:

> Dave Hansen wrote:
>
...

>>
>> I think that stack allocation is a pretty nasty trick for a structure
>> that's supposed to be pretty persistent and dynamically allocated, and
>> is certainly something that needs to get fixed up in a proper way.
>>
>>
>
> agreed.
>
>> This works around the problem for now, but this could potentially cause
>> more bugs any time we add some member to 'struct file' and depend on its
>> state being sane anywhere in the VFS. If there's a list anywhere of
>> merge-stopper reiser4 bugs around, this should probably go in there.
>>
>>
>
> will be fixed.
>

The promised fixup is attached.
Andrew, please apply.

Thanks,
Edward.

[-- Attachment #2: reiser4-fix-null-dereference-in-__mnt_is_readonly-in-ftruncate-2.patch --]
[-- Type: text/x-patch, Size: 5400 bytes --]

Do not allocate struct file on stack, pass the persistent one instead.

Signed-off-by: Edward Shishkin <edward@namesys.com>
---
 linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/file.c            |   35 ++++------
 linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/file.h            |    2 
 linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/tail_conversion.c |   23 ++----
 3 files changed, 26 insertions(+), 34 deletions(-)

--- linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/file.c.orig
+++ linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/file.c
@@ -566,23 +566,18 @@
  * items or add them to represent a hole at the end of file. The caller has to
  * obtain exclusive access to the file.
  */
-static int truncate_file_body(struct inode *inode, loff_t new_size)
+static int truncate_file_body(struct inode *inode, struct iattr *attr)
 {
 	int result;
+	loff_t new_size = attr->ia_size;
 
 	if (inode->i_size < new_size) {
 		/* expanding truncate */
-		struct dentry dentry;
-		struct file file;
-		struct unix_file_info *uf_info;
+		struct file * file = attr->ia_file;
+		struct unix_file_info *uf_info = unix_file_inode_data(inode);
+
+		assert("edward-1532", attr->ia_valid & ATTR_FILE);
 
-		dentry.d_inode = inode;
-		file.f_dentry = &dentry;
-		file.private_data = NULL;
-		file.f_pos = new_size;
-		file.private_data = NULL;
-		file.f_vfsmnt = NULL;
-		uf_info = unix_file_inode_data(inode);
 		result = find_file_state(inode, uf_info);
 		if (result)
 			return result;
@@ -615,19 +610,19 @@
 						return result;
 				}
 			}
-			result = reiser4_write_extent(&file, NULL, 0,
+			result = reiser4_write_extent(file, NULL, 0,
 						      &new_size);
 			if (result)
 				return result;
 			uf_info->container = UF_CONTAINER_EXTENTS;
 		} else {
 			if (uf_info->container ==  UF_CONTAINER_EXTENTS) {
-				result = reiser4_write_extent(&file, NULL, 0,
+				result = reiser4_write_extent(file, NULL, 0,
 							      &new_size);
 				if (result)
 					return result;
 			} else {
-				result = reiser4_write_tail(&file, NULL, 0,
+				result = reiser4_write_tail(file, NULL, 0,
 							    &new_size);
 				if (result)
 					return result;
@@ -636,10 +631,10 @@
 		}
 		BUG_ON(result > 0);
 		INODE_SET_FIELD(inode, i_size, new_size);
-		file_update_time(&file);
+		file_update_time(file);
 		result = reiser4_update_sd(inode);
 		BUG_ON(result != 0);
-		reiser4_free_file_fsdata(&file);
+		reiser4_free_file_fsdata(file);
 	} else
 		result = shorten_file(inode, new_size);
 	return result;
@@ -2092,7 +2087,7 @@
 		 * first item is formatting item, therefore there was
 		 * incomplete extent2tail conversion. Complete it
 		 */
-		result = extent2tail(unix_file_inode_data(inode));
+		result = extent2tail(file, unix_file_inode_data(inode));
 	else
 		result = -EIO;
 
@@ -2372,7 +2367,7 @@
 		    uf_info->container == UF_CONTAINER_EXTENTS &&
 		    !should_have_notail(uf_info, inode->i_size) &&
 		    !rofs_inode(inode)) {
-			result = extent2tail(uf_info);
+			result = extent2tail(file, uf_info);
 			if (result != 0) {
 				warning("nikita-3233",
 					"Failed (%d) to convert in %s (%llu)",
@@ -2638,7 +2633,7 @@
 	if (result == 0)
 		result = safe_link_add(inode, SAFE_TRUNCATE);
 	if (result == 0)
-		result = truncate_file_body(inode, attr->ia_size);
+		result = truncate_file_body(inode, attr);
 	if (result)
 		warning("vs-1588", "truncate_file failed: oid %lli, "
 			"old size %lld, new size %lld, retval %d",
@@ -2724,7 +2719,7 @@
 	/* truncate file bogy first */
 	uf_info = unix_file_inode_data(inode);
 	get_exclusive_access(uf_info);
-	result = truncate_file_body(inode, 0 /* size */ );
+	result = shorten_file(inode, 0 /* size */ );
 	drop_exclusive_access(uf_info);
 
 	if (result)
--- linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/file.h.orig
+++ linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/file.h
@@ -237,7 +237,7 @@
 #define WRITE_GRANULARITY 32
 
 int tail2extent(struct unix_file_info *);
-int extent2tail(struct unix_file_info *);
+int extent2tail(struct file *, struct unix_file_info *);
 
 int goto_right_neighbor(coord_t *, lock_handle *);
 int find_or_create_extent(struct page *);
--- linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/tail_conversion.c.orig
+++ linux-2.6.23-rc8-mm2/fs/reiser4/plugin/file/tail_conversion.c
@@ -546,7 +546,7 @@
 
 /* for every page of file: read page, cut part of extent pointing to this page,
    put data of page tree by tail item */
-int extent2tail(struct unix_file_info *uf_info)
+int extent2tail(struct file * file, struct unix_file_info *uf_info)
 {
 	int result;
 	struct inode *inode;
@@ -644,20 +644,17 @@
 			/* last page can be incompleted */
 			count = (inode->i_size & ~PAGE_CACHE_MASK);
 		while (count) {
-			struct dentry dentry;
-			struct file file;
-			loff_t pos;
-
-			dentry.d_inode = inode;
-			file.f_dentry = &dentry;
-			file.private_data = NULL;
-			file.f_pos = start_byte;
-			file.private_data = NULL;
-			pos = start_byte;
-			result = reiser4_write_tail(&file,
+			loff_t pos = start_byte;
+
+			assert("edward-1533",
+			       file != NULL && file->f_dentry != NULL);
+			assert("edward-1534",
+			       file->f_dentry->d_inode == inode);
+
+			result = reiser4_write_tail(file,
 						    (char __user *)kmap(page),
 						    count, &pos);
-			reiser4_free_file_fsdata(&file);
+			reiser4_free_file_fsdata(file);
 			if (result <= 0) {
 				warning("", "reiser4_write_tail failed");
 				page_cache_release(page);

  reply	other threads:[~2007-10-04 23:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-27 21:54 2.6.23-rc8-mm2 NULL dereference in __mnt_is_readonly in ftruncate Zan Lynx
2007-09-28  8:30 ` Andrew Morton
2007-09-28 16:21   ` Dave Hansen
2007-09-28 22:39     ` Edward Shishkin
2007-10-04 23:22       ` Edward Shishkin [this message]
2007-10-05 16:22         ` [patch] reiser4: do not allocate struct file on stack Zan Lynx

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=470575AE.9060902@namesys.com \
    --to=edward@namesys.com \
    --cc=akpm@linux-foundation.org \
    --cc=haveblue@us.ibm.com \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=reiserfs-devel@vger.kernel.org \
    --cc=vs@clusterfs.com \
    --cc=zlynx@acm.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox