mmap.c (do_mmap_pgoff), against 2.4.19 and 2.4.20-pre10

Hi all, specially Marcelo :)

This is the fourth and last time I submit this patch to Marcelo.
This little tiny bug is fixed in all trees except the official one. I think this patch is trivial enough to be accepted, but...

Well, the attachments included (unified diff format), is the patch
against both 2.4.19 and 2.4.20-pre10 (I've changed the kernel name directory part to '/usr/src/linux/' so it's applicable to both versions.

Marcelo, if you don't want to include this patch at least let me
know, please, so I won't need to see each new prerelease for seeing if the patch has been already included ;))) Don't take it bad.

Raúl

@@ -390,6 +390,12 @@
 	return 0;
 }
 
+
+/*
+ *	NOTE: in this function we rely on TASK_SIZE being lower than
+ *	SIZE_MAX-PAGE_SIZE at least. I'm pretty sure that it is.
+ */
+
 unsigned long do_mmap_pgoff(struct file * file, unsigned long addr, unsigned long len,
 	unsigned long prot, unsigned long flags, unsigned long pgoff)
 {

@@ -403,12 +409,14 @@
 	if (file && (!file->f_op || !file->f_op->mmap))
 		return -ENODEV;
 
-	if ((len = PAGE_ALIGN(len)) == 0)
+	if (!len)
 		return addr;
 
 	if (len > TASK_SIZE)
 		return -EINVAL;
 
+	len = PAGE_ALIGN(len);  /* This cannot be zero now */
+
 	/* offset overflow? */
 	if ((pgoff + (len >> PAGE_SHIFT)) < pgoff)
 		return -EINVAL;

From: DervishD <[email protected]>
Date: Mon, 14 Oct 2002 11:36:22 +0200

This is the fourth and last time I submit this patch to Marcelo.
This little tiny bug is fixed in all trees except the official one. I think this patch is trivial enough to be accepted, but...

Patches tend to get accepted when you attach an analysis of the bug you are fixing.

I cannot even figure out what the failure case is that you are fixing which actually occurs.

I bet if you explain this, Marcelo will take your fix.

    Hi Davem :)))

I sent that description the three times before. In fact, you have
this patch in your tree (or is -dj tree? If I'm wrong please my excuses...). In fact, this patch is in the -aa tree (I think), in 2.2, 2.5 and in the -ac series. I don't know if more people has included it, because I sent to LKML and some people adopted the patch.

The bug is that when you specify a size to mmap() whose last
address is in the last page of the addressable space for the process, the 'PAGE_ALIGN()' macro converts a *size* to an *address* of '0' because there is no way of telling the macro 'hey, this is not an address, but a size, align up to a multiple of the page size, but don't set it to '0' if it's too big for you'. The problem is that the alignment takes place before the checking for limits. If the size is greater than the addressable size the function should return -EINVAL, not '0'...

Now, if the size requested is '0', the hint address is returned,
and if the size is larger than TASK_SIZE, it fails with '-EINVAL'. Now if you are checking (prior to compiling something, for example) what is the larger chunk of 'mmapable' memory you can get, you can do it safely. Or just you need to map a big file and you're trying to do
it: you need to know if mmaping such a large size is failing or not, but without the patch mmap() just returns the hint address...

Marcelo told me to resend this patch at 2.4.20-pre time, and I
did. I'm not telling that Marcelo has dropped this patch arbitrarily. Maybe he doesn't rely on it, and it's good, patches shouldn't get into the kernel without reason, but I resent two times at 2.4.20-pre time and I don't know why it is not included. And it's a bug that affects me ;))) Maybe messages didn't arrive, maybe I did the patch wrong, who knows... I'm not attacking Marcelo nor am I angry ;)) but I'm not guilty, neither ;)))

Raúl

From: DervishD <[email protected]>
Date: Mon, 14 Oct 2002 12:20:42 +0200

Marcelo told me to resend this patch at 2.4.20-pre time,

Thanks for describing your fix all over again, and I hope this leads Marcelo to inspect your fix more closely :)

The patch is very stupid, just as the bug ;)) Really is only one
line added and another one moved ;)) I hope this time the patch is ok. I've resent it with the suggestions from Russell King :)

Raúl

You could try sending a patch that conforms to the following (why don't we include a comment about this in Documentation/SubmittingPatches?)

This would remove an extra reason why your patch may not have been accepted.

PATCH(1) PATCH(1)

NAME
patch - apply a diff file to an original
...
NOTES FOR PATCH SENDERS
There are several things you should bear in mind if you are going to be sending out patches.
...
If the recipient is supposed to use the -pN option, do not send output that looks like this:

diff -Naur v2.0.29/prog/README prog/README
--- v2.0.29/prog/README Mon Mar 10 15:13:12 1997
+++ prog/README Mon Mar 17 14:58:22 1997

because the two file names have different numbers of slashes, and different versions of patch interpret the file names differently.

Content-Description: mmap.c.diff

Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Hi Russell :))

Sorry, was a typo, you're right...

Since I do not consider this to be critical, I _will_ apply it on 2.4.21-pre.

At that time, resend me without the addition of the comment.

Thanks and sorry for being so unresponsive

Hi Marcelo :)

OK, but you told me just the same when I sent you the patch at 19
pre time... After all, there are just two lines of code...

    Well, I'll do...

Not at all, Marcelo. As I told, don't take it bad ;) Raúl