All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.