mtdchar.c: Fix regression in MEMGETREGIONINFO ioctl()

PATCH In review Zev Weiss 4 participants 11 messages Aug 20, 2008 – Sep 1, 2008 Memory technology devices (MTD)

lore

Share & tools

lore

Zev Weiss

author

2008-08-20 07:47 UTC #0 lore raw

PATCH 1/1 mtdchar.c: Fix regression in MEMGETREGIONINFO ioctl()

From: Zev Weiss <[email protected]>

The MEMGETREGIONINFO ioctl() in mtdchar.c was clobbering user memory by overwriting more than intended, due to the size of struct mtd_erase_region_info changing in commit 0ecbc81adfcb9f15f86b05ff576b342ce81bbef8.

Fix uses a member-by-member copy into a local struct region_info_user, which is then copy_to_user()'d (and matches the size correctly by being of the same type as the pointer passed in the ioctl() call).

Signed-off-by Zev Weiss <[email protected]>

Tested-by Zev Weiss <[email protected]>

---
I had been having some problems with userspace memory corruption, and traced them to a MEMGETREGIONINFO ioctl() on an MTD device. I applied this patch and it seems to fix the problem, though I am not an expert and there may be a more correct way to go about doing this. I'm also new at submitting patches, so hopefully I haven't screwed up the patch-submission etiquette too horrifically.

 drivers/mtd/mtdchar.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

drivers/mtd/mtdchar.c +9 -2

index 13cc67a..0acb135 100644
@@ -411,14 +411,21 @@ static int mtd_ioctl(struct inode *inode, struct file *file,
 	case MEMGETREGIONINFO:
 	{
 		struct region_info_user ur;
+		struct mtd_erase_region_info *kr;
 
 		if (copy_from_user(&ur, argp, sizeof(struct region_info_user)))
 			return -EFAULT;
 
 		if (ur.regionindex >= mtd->numeraseregions)
 			return -EINVAL;
-		if (copy_to_user(argp, &(mtd->eraseregions[ur.regionindex]),
-				sizeof(struct mtd_erase_region_info)))
+
+		kr = &(mtd->eraseregions[ur.regionindex]);
+
+		ur.offset = kr->offset;
+		ur.erasesize = kr->erasesize;
+		ur.numblocks = kr->numblocks;
+
+		if (copy_to_user(argp, &ur, sizeof(struct region_info_user)))
 			return -EFAULT;
 		break;
 	}

Andrew Morton

2008-08-22 22:34 UTC #1 lore raw

Quoted 42 lines · On Wed, 20 Aug 2008 00:47:23 -0700 Zev Weiss <[email protected]> wrote:

From: Zev Weiss <[email protected]>
The MEMGETREGIONINFO ioctl() in mtdchar.c was clobbering user memory by overwriting more than intended, due to the size of struct mtd_erase_region_info changing in commit 0ecbc81adfcb9f15f86b05ff576b342ce81bbef8.
Fix uses a member-by-member copy into a local struct region_info_user, which is then copy_to_user()'d (and matches the size correctly by being of the same type as the pointer passed in the ioctl() call).
Signed-off-by Zev Weiss <[email protected]>
Tested-by Zev Weiss <[email protected]>
---
I had been having some problems with userspace memory corruption, and traced them to a MEMGETREGIONINFO ioctl() on an MTD device. I applied this patch and it seems to fix the problem, though I am not an expert and there may be a more correct way to go about doing this. I'm also new at submitting patches, so hopefully I haven't screwed up the patch-submission etiquette too horrifically.
  drivers/mtd/mtdchar.c |   11 +++++++++--
  1 files changed, 9 insertions(+), 2 deletions(-)
drivers/mtd/mtdchar.c +2 -2
index 13cc67a..0acb135 100644
@@ -411,14 +411,21 @@ static int mtd_ioctl(struct inode *inode, struct file *file,
  	case MEMGETREGIONINFO:
  	{
  		struct region_info_user ur;
+		struct mtd_erase_region_info *kr;
 
  		if (copy_from_user(&ur, argp, sizeof(struct region_info_user)))
  			return -EFAULT;
 
  		if (ur.regionindex >= mtd->numeraseregions)
  			return -EINVAL;
-		if (copy_to_user(argp, &(mtd->eraseregions[ur.regionindex]),
-				sizeof(struct mtd_erase_region_info)))
+

drivers/mtd/mtdchar.c static int mtd_ioctl(struct inode *inode, struct file *file, · line 431

Quoted 10 lines

drivers/mtd/mtdchar.c +7 -0

+		kr = &(mtd->eraseregions[ur.regionindex]);
+
+		ur.offset = kr->offset;
+		ur.erasesize = kr->erasesize;
+		ur.numblocks = kr->numblocks;
+
+		if (copy_to_user(argp, &ur, sizeof(struct region_info_user)))
  			return -EFAULT;
  		break;
  	}

ug.

Putting a kernel pointer into a shared-with-userspace data structure (struct mtd_erase_region_info) was a big mistake.

Copying a struct region_info_user' back to userspace seems better than copying a struct mtd_erase_region_info', but what do I know?

Actually...

Before 0ecbc81adfcb9f15f86b05ff576b342ce81bbef8, `struct mtd_erase_region_info' had three members, all u32. We were copying three u32's out to userspace.

After 0ecbc81adfcb9f15f86b05ff576b342ce81bbef8, `struct mtd_erase_region_info' has four members: three u32's and one ulong*. We're copying three u32's and one ulong* out to userspace.

After your change, we're copying _four_ u32's out to userspace, so there still is potential for scribbling on unsuspecting userspace?

If that reading is right, we need to go back to copying just the three u32's. Perhaps via

struct mtd_erase_region_info {
	struct {
		u_int32_t offset;
		u_int32_t erasesize;
		u_int32_t numblocks;
	} user_part;
	unsigned long *lockmap;
};

or similar.

David? Help? 2.6.25.x anmd 2.6.26.x need fixing as well.

Zev Weiss

author

2008-08-23 08:10 UTC #2 lore raw

Quoted 88 lines · Andrew Morton

Quoted 42 lines · On Wed, 20 Aug 2008 00:47:23 -0700 Zev Weiss <[email protected]> wrote:
From: Zev Weiss <[email protected]>
The MEMGETREGIONINFO ioctl() in mtdchar.c was clobbering user memory by overwriting more than intended, due to the size of struct mtd_erase_region_info changing in commit 0ecbc81adfcb9f15f86b05ff576b342ce81bbef8.
Fix uses a member-by-member copy into a local struct region_info_user, which is then copy_to_user()'d (and matches the size correctly by being of the same type as the pointer passed in the ioctl() call).
Signed-off-by Zev Weiss <[email protected]>
Tested-by Zev Weiss <[email protected]>
---
I had been having some problems with userspace memory corruption, and traced them to a MEMGETREGIONINFO ioctl() on an MTD device. I applied this patch and it seems to fix the problem, though I am not an expert and there may be a more correct way to go about doing this. I'm also new at submitting patches, so hopefully I haven't screwed up the patch-submission etiquette too horrifically.
  drivers/mtd/mtdchar.c |   11 +++++++++--
  1 files changed, 9 insertions(+), 2 deletions(-)
drivers/mtd/mtdchar.c +2 -2
index 13cc67a..0acb135 100644
@@ -411,14 +411,21 @@ static int mtd_ioctl(struct inode *inode, struct file *file,
  	case MEMGETREGIONINFO:
  	{
  		struct region_info_user ur;
+		struct mtd_erase_region_info *kr;
 
  		if (copy_from_user(&ur, argp, sizeof(struct region_info_user)))
  			return -EFAULT;
 
  		if (ur.regionindex >= mtd->numeraseregions)
  			return -EINVAL;
-		if (copy_to_user(argp, &(mtd->eraseregions[ur.regionindex]),
-				sizeof(struct mtd_erase_region_info)))
+
drivers/mtd/mtdchar.c static int mtd_ioctl(struct inode *inode, struct file *file, · line 431
Quoted 10 lines
drivers/mtd/mtdchar.c +7 -0
+		kr = &(mtd->eraseregions[ur.regionindex]);
+
+		ur.offset = kr->offset;
+		ur.erasesize = kr->erasesize;
+		ur.numblocks = kr->numblocks;
+
+		if (copy_to_user(argp, &ur, sizeof(struct region_info_user)))
  			return -EFAULT;
  		break;
  	}
ug.
Putting a kernel pointer into a shared-with-userspace data structure (struct mtd_erase_region_info) was a big mistake.
Copying a struct region_info_user' back to userspace seems better than copying a struct mtd_erase_region_info', but what do I know?
Actually...
Before 0ecbc81adfcb9f15f86b05ff576b342ce81bbef8, `struct mtd_erase_region_info' had three members, all u32. We were copying three u32's out to userspace.
After 0ecbc81adfcb9f15f86b05ff576b342ce81bbef8, `struct
mtd_erase_region_info' has four members: three u32's and one ulong*. We're copying three u32's and one ulong* out to userspace.
After your change, we're copying _four_ u32's out to userspace, so there still is potential for scribbling on unsuspecting userspace?
If that reading is right, we need to go back to copying just the three u32's. Perhaps via
struct mtd_erase_region_info {
	struct {
		u_int32_t offset;
		u_int32_t erasesize;
		u_int32_t numblocks;
	} user_part;
	unsigned long *lockmap;
};

Quoted 5 lines

or similar.
David? Help? 2.6.25.x anmd 2.6.26.x need fixing as well.

Hmm. Well, I may be misunderstanding what you're saying (again, I'm very much a newbie to kernelspace), but I *think* the "copying four u32's out to userspace" thing isn't really a problem with my patch. It does certainly copy those four u32's, but given that ur' (struct mtd_region_info_user) is initialized by copying from userspace, its fourth u32 (the regionindex' member) should be identical when copied back out to userspace, given that it's not touched in the memberwise modification of the struct. So yes, it is copying 4 bytes more than is strictly necessary, but it seemed like a reasonably clean way of going about it (to me, for what that's worth).

In my particular situation it didn't do anything unexpected in my testing (and restored the normal behavior I had when previously running 2.6.17.7).

On the other hand, if I'm missing something completely, please let me know, and perhaps I can prepare a more suitable fix.

Thanks,
Zev

Andrew Morton

2008-08-24 05:27 UTC #3 lore raw

Quoted 96 lines · On Sat, 23 Aug 2008 01:10:21 -0700 Zev Weiss <[email protected]> wrote:

Quoted 89 lines · Andrew Morton
Quoted 42 lines · On Wed, 20 Aug 2008 00:47:23 -0700 Zev Weiss <[email protected]> wrote:
From: Zev Weiss <[email protected]>
The MEMGETREGIONINFO ioctl() in mtdchar.c was clobbering user memory by overwriting more than intended, due to the size of struct mtd_erase_region_info changing in commit 0ecbc81adfcb9f15f86b05ff576b342ce81bbef8.
Fix uses a member-by-member copy into a local struct region_info_user, which is then copy_to_user()'d (and matches the size correctly by being of the same type as the pointer passed in the ioctl() call).
Signed-off-by Zev Weiss <[email protected]>
Tested-by Zev Weiss <[email protected]>
---
I had been having some problems with userspace memory corruption, and traced them to a MEMGETREGIONINFO ioctl() on an MTD device. I applied this patch and it seems to fix the problem, though I am not an expert and there may be a more correct way to go about doing this. I'm also new at submitting patches, so hopefully I haven't screwed up the patch-submission etiquette too horrifically.
  drivers/mtd/mtdchar.c |   11 +++++++++--
  1 files changed, 9 insertions(+), 2 deletions(-)
drivers/mtd/mtdchar.c +2 -2
index 13cc67a..0acb135 100644
@@ -411,14 +411,21 @@ static int mtd_ioctl(struct inode *inode, struct file *file,
  	case MEMGETREGIONINFO:
  	{
  		struct region_info_user ur;
+		struct mtd_erase_region_info *kr;
 
  		if (copy_from_user(&ur, argp, sizeof(struct region_info_user)))
  			return -EFAULT;
 
  		if (ur.regionindex >= mtd->numeraseregions)
  			return -EINVAL;
-		if (copy_to_user(argp, &(mtd->eraseregions[ur.regionindex]),
-				sizeof(struct mtd_erase_region_info)))
+
drivers/mtd/mtdchar.c static int mtd_ioctl(struct inode *inode, struct file *file, · line 431
Quoted 10 lines
drivers/mtd/mtdchar.c +7 -0
+		kr = &(mtd->eraseregions[ur.regionindex]);
+
+		ur.offset = kr->offset;
+		ur.erasesize = kr->erasesize;
+		ur.numblocks = kr->numblocks;
+
+		if (copy_to_user(argp, &ur, sizeof(struct region_info_user)))
  			return -EFAULT;
  		break;
  	}
ug.
Putting a kernel pointer into a shared-with-userspace data structure (struct mtd_erase_region_info) was a big mistake.
Copying a struct region_info_user' back to userspace seems better than copying a struct mtd_erase_region_info', but what do I know?
Actually...
Before 0ecbc81adfcb9f15f86b05ff576b342ce81bbef8, `struct mtd_erase_region_info' had three members, all u32. We were copying three u32's out to userspace.
After 0ecbc81adfcb9f15f86b05ff576b342ce81bbef8, `struct mtd_erase_region_info' has four members: three u32's and one ulong*. We're copying three u32's and one ulong* out to userspace.
After your change, we're copying _four_ u32's out to userspace, so there still is potential for scribbling on unsuspecting userspace?
If that reading is right, we need to go back to copying just the three u32's. Perhaps via
struct mtd_erase_region_info {
	struct {
		u_int32_t offset;
		u_int32_t erasesize;
		u_int32_t numblocks;
	} user_part;
	unsigned long *lockmap;
};
Quoted 5 lines
or similar.
David? Help? 2.6.25.x anmd 2.6.26.x need fixing as well.

Quoted 7 lines

Hmm. Well, I may be misunderstanding what you're saying (again, I'm very much a newbie to kernelspace), but I *think* the "copying four u32's out to userspace" thing isn't really a problem with my patch. It does certainly copy those four u32's, but given that ur' (struct mtd_region_info_user) is initialized by copying from userspace, its fourth u32 (the regionindex' member) should be identical when copied back out to userspace, given that it's not touched in the memberwise modification of the struct.

OK, that's fortuitously bug-free in single-threaded userspace but fantastically-improbably-buggy if userspace is threaded.

But it's something the kernel shouldn't be doing.

Quoted 9 lines

So yes, it is
copying 4 bytes more than is strictly necessary, but it seemed like a reasonably clean way of going about it (to me, for what that's worth).
In my particular situation it didn't do anything unexpected in my testing (and restored the normal behavior I had when previously running 2.6.17.7).
On the other hand, if I'm missing something completely, please let me know, and perhaps I can prepare a more suitable fix.

"good enough" is never good enough ;)

What is the ideal implementation? Let's implement that.

Zev Weiss

author

2008-08-24 11:01 UTC #4 lore raw

Quoted 10 lines · Andrew Morton

Quoted 7 lines · On Sat, 23 Aug 2008 01:10:21 -0700 Zev Weiss <[email protected]> wrote:
Hmm. Well, I may be misunderstanding what you're saying (again, I'm very much a newbie to kernelspace), but I *think* the "copying four u32's out to userspace" thing isn't really a problem with my patch. It does certainly copy those four u32's, but given that ur' (struct mtd_region_info_user) is initialized by copying from userspace, its fourth u32 (the regionindex' member) should be identical when copied back out to userspace, given that it's not touched in the memberwise modification of the struct.

Quoted 5 lines

OK, that's fortuitously bug-free in single-threaded userspace but fantastically-improbably-buggy if userspace is threaded.
But it's something the kernel shouldn't be doing.

Ah, good point -- that hadn't occurred to me at all. Though it looks pretty
clumsy/simpleminded to me, I guess something like this would avoid copying and rewriting the fourth u32 ("Well, duh" may be the appropriate response here):

drivers/mtd/mtdchar.c +10 -6

index 13cc67a..424f318 100644
@@ -410,16 +410,20 @@ static int mtd_ioctl(struct inode *inode, struct file *file,
 
        case MEMGETREGIONINFO:
        {
-               struct region_info_user ur;
+               u32 ur_idx;
+               struct mtd_erase_region_info *kr;
+               struct region_info_user *ur = (struct region_info_user *) argp;
 
-               if (copy_from_user(&ur, argp, sizeof(struct region_info_user)))
+               if (get_user(ur_idx,&(ur->regionindex)))
                        return -EFAULT;
 
-               if (ur.regionindex >= mtd->numeraseregions)
-                       return -EINVAL;
-               if (copy_to_user(argp, &(mtd->eraseregions[ur.regionindex]),
-                               sizeof(struct mtd_erase_region_info)))
+               kr = &(mtd->eraseregions[ur_idx]);
+
+               if (put_user(kr->offset, &(ur->offset))
+                   || put_user(kr->erasesize, &(ur->erasesize))
+                   || put_user(kr->numblocks, &(ur->numblocks)))
                        return -EFAULT;
+
                break;
        }

[Note: this is not even so much as compile-tested, and will remain that way until Monday, but I think you get the picture.]

Quoted 7 lines

Quoted 7 lines
So yes, it is
copying 4 bytes more than is strictly necessary, but it seemed like a reasonably clean way of going about it (to me, for what that's worth).
In my particular situation it didn't do anything unexpected in my testing (and restored the normal behavior I had when previously running 2.6.17.7).

Quoted 8 lines

Quoted 2 lines
On the other hand, if I'm missing something completely, please let me know, and perhaps I can prepare a more suitable fix.
"good enough" is never good enough ;)
What is the ideal implementation? Let's implement that.

Well that, I'm afraid, is a question that I think would require somewhat greater depth of understanding than I possess (and I very much doubt the above patch is it). Given what you said earlier, it seems like the ideal solution would involve a reworking of the patch that added the `lockmap' member to struct mtd_erase_region_info, but I'm probably not the person to be messing around with that. So...anyone-who-knows-mtd-better-than-I, care to comment?

Jamie Lokier

2008-08-24 12:38 UTC #10 lore raw

In reply to Andrew Morton

Quoted 8 lines · Andrew Morton

struct mtd_erase_region_info {
	struct {
		u_int32_t offset;
		u_int32_t erasesize;
		u_int32_t numblocks;
	} user_part;
	unsigned long *lockmap;
};

Is the name "struct mtd_erase_region_info" something userspace uses?

Nothing wrong with changing the kernel's version, as long as userspace uses different headers, but it might be confusing.

-- Jamie

David Woodhouse

2008-08-25 11:37 UTC #9 lore raw

In reply to Andrew Morton

Quoted 7 lines · Andrew Morton

Quoted 3 lines
On the other hand, if I'm missing something completely, please let me know, and perhaps I can prepare a more suitable fix.
"good enough" is never good enough ;)
What is the ideal implementation? Let's implement that.

It involves ditching the ioctls altogether (well, not adding anything _more_ to them, at least) and doing it through sysfs.

Signature · 2 lines

David Woodhouse                            Open Source Technology Centre
[email protected]                              Intel Corporation

Zev Weiss

author

2008-08-27 04:31 UTC #5 lore raw

In reply to Zev Weiss

Quoted 60 lines · Zev Weiss

Quoted 3 lines · Andrew Morton
Quoted 1 line · On Sat, 23 Aug 2008 01:10:21 -0700 Zev Weiss <[email protected]> wrote:
Hmm. Well, I may be misunderstanding what you're saying (again, I'm
very much
Quoted 2 lines
Quoted 2 lines
a newbie to kernelspace), but I *think* the "copying four u32's out to userspace" thing isn't really a problem with my patch. It does
certainly copy
Quoted 2 lines
Quoted 2 lines
those four u32's, but given that `ur' (struct mtd_region_info_user) is initialized by copying from userspace, its fourth u32 (the
`regionindex'
Quoted 1 line
Quoted 1 line
member) should be identical when copied back out to userspace, given
that it's
Quoted 7 lines
Quoted 1 line
not touched in the memberwise modification of the struct.
OK, that's fortuitously bug-free in single-threaded userspace but fantastically-improbably-buggy if userspace is threaded.
But it's something the kernel shouldn't be doing.
Ah, good point -- that hadn't occurred to me at all. Though it looks pretty
clumsy/simpleminded to me, I guess something like this would avoid copying and rewriting the fourth u32 ("Well, duh" may be the appropriate response here):
drivers/mtd/mtdchar.c +10 -6
index 13cc67a..424f318 100644
@@ -410,16 +410,20 @@ static int mtd_ioctl(struct inode *inode, struct 
 file *file,
 
        case MEMGETREGIONINFO:
        {
-               struct region_info_user ur;
+               u32 ur_idx;
+               struct mtd_erase_region_info *kr;
+               struct region_info_user *ur = (struct region_info_user 
 *) argp;
 
-               if (copy_from_user(&ur, argp, sizeof(struct 
 region_info_user)))
+               if (get_user(ur_idx,&(ur->regionindex)))
                        return -EFAULT;
 
-               if (ur.regionindex >= mtd->numeraseregions)
-                       return -EINVAL;
-               if (copy_to_user(argp, 
 &(mtd->eraseregions[ur.regionindex]),
-                               sizeof(struct mtd_erase_region_info)))
+               kr = &(mtd->eraseregions[ur_idx]);
+
+               if (put_user(kr->offset, &(ur->offset))
+                   || put_user(kr->erasesize, &(ur->erasesize))
+                   || put_user(kr->numblocks, &(ur->numblocks)))
                        return -EFAULT;
+
                break;
        }

Quoted 3 lines

[Note: this is not even so much as compile-tested, and will remain that way until Monday, but I think you get the picture.]

Well, for what little it's probably worth, I've now tested this patch, with no resulting surprises (seems to work fine for me). Though I see now that, much to my chagrin, something in my email chain seems to have spacified my code. If there's any need I can resend appropriately.

Zev

David Woodhouse

2008-09-01 11:01 UTC #6 lore raw

Quoted 4 lines · Zev Weiss

Well, for what little it's probably worth, I've now tested this patch, with no resulting surprises (seems to work fine for me). Though I see now that, much to my chagrin, something in my email chain seems to have spacified my code. If there's any need I can resend appropriately.

Please. With a signed-off-by.

Signature · 2 lines

David Woodhouse                            Open Source Technology Centre
[email protected]                              Intel Corporation

Zev Weiss

author

2008-09-01 12:19 UTC #7 lore raw

From: Zev Weiss <[email protected]>
Date: Mon, 1 Sep 2008 05:02:12 -0700
Subject: [PATCH] [MTD] mtdchar.c: Fix regression in MEMGETREGIONINFO ioctl()

The MEMGETREGIONINFO ioctl() in mtdchar.c was clobbering user memory by overwriting more than intended, due the size of struct mtd_erase_region_info changing in commit 0ecbc81adfcb9f15f86b05ff576b342ce81bbef8.

Fix avoids this by copying struct members one by one with put_user(), as there is no longer a convenient struct to use the size of as the length argument to copy_to_user().

Signed-off-by Zev Weiss <[email protected]>

---
 drivers/mtd/mtdchar.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

drivers/mtd/mtdchar.c +10 -6

index 13cc67a..424f318 100644
@@ -410,16 +410,20 @@ static int mtd_ioctl(struct inode *inode, struct file *file,
 
 	case MEMGETREGIONINFO:
 	{
-		struct region_info_user ur;
+		u32 ur_idx;
+		struct mtd_erase_region_info *kr;
+		struct region_info_user *ur = (struct region_info_user *) argp;
 
-		if (copy_from_user(&ur, argp, sizeof(struct region_info_user)))
+		if (get_user(ur_idx,&(ur->regionindex)))
 			return -EFAULT;
 
-		if (ur.regionindex >= mtd->numeraseregions)
-			return -EINVAL;
-		if (copy_to_user(argp, &(mtd->eraseregions[ur.regionindex]),
-				sizeof(struct mtd_erase_region_info)))
+		kr = &(mtd->eraseregions[ur_idx]);
+
+		if (put_user(kr->offset, &(ur->offset))
+		    || put_user(kr->erasesize, &(ur->erasesize))
+		    || put_user(kr->numblocks, &(ur->numblocks)))
 			return -EFAULT;
+
 		break;
 	}

Signature · 1 line

1.5.4.1

David Woodhouse

2008-09-01 17:25 UTC #8 lore raw

Quoted 8 lines · Zev Weiss

From: Zev Weiss <[email protected]>
Date: Mon, 1 Sep 2008 05:02:12 -0700
Subject: [PATCH] [MTD] mtdchar.c: Fix regression in MEMGETREGIONINFO ioctl()
The MEMGETREGIONINFO ioctl() in mtdchar.c was clobbering user memory by overwriting more than intended, due the size of struct mtd_erase_region_info changing in commit 0ecbc81adfcb9f15f86b05ff576b342ce81bbef8.

Quoted 5 lines

Fix avoids this by copying struct members one by one with put_user(), as there is no longer a convenient struct to use the size of as the length argument to copy_to_user().
Signed-off-by Zev Weiss <[email protected]>

Thanks. Your patch was whitespace-damaged, but I managed to apply it anyway. Please check for future patches though -- try sending patches to yourself and then see if they get mangled. I believe gmail is known to be broken unless you submit mail with SMTP.

I also try to avoid the pointless types like 'u32' in MTD code -- the C language has perfectly good explicitly sized types; let's use them.

Signature · 2 lines

David Woodhouse                            Open Source Technology Centre
[email protected]                              Intel Corporation