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

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

[DervishD]

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

    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

[-- Attachment #2: mmap.c.diff --]
[-- Type: text/plain, Size: 824 bytes --]

--- /usr/src/linux/mm/mmap.c.orig	2002-10-14 11:16:40.000000000 +0200
+++ /usr/src/linux/kernel/mm/mmap.c	2002-10-14 11:19:32.000000000 +0200
@@ -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;

Re: [PATCH] mmap.c (do_mmap_pgoff), against 2.4.19 and 2.4.20-pre10

[David S. Miller]

   From: DervishD <raul@pleyades.net>
   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.

Re: [PATCH] mmap.c (do_mmap_pgoff), against 2.4.19 and 2.4.20-pre10

[DervishD]

    Hi Davem :)))

>        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 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.

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

    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...

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

    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

Re: [PATCH] mmap.c (do_mmap_pgoff), against 2.4.19 and 2.4.20-pre10

[David S. Miller]

   From: DervishD <raul@pleyades.net>
   Date: Mon, 14 Oct 2002 12:20:42 +0200

   > I bet if you explain this, Marcelo will take your fix.
   
       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 :)

Re: [PATCH] mmap.c (do_mmap_pgoff), against 2.4.19 and 2.4.20-pre10

[DervishD]

>    > I bet if you explain this, Marcelo will take your fix.
>        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

Re: [PATCH] mmap.c (do_mmap_pgoff), against 2.4.19 and 2.4.20-pre10

[Russell King]

On Mon, Oct 14, 2002 at 11:36:22AM +0200, DervishD wrote:
>     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.

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
> --- /usr/src/linux/mm/mmap.c.orig	2002-10-14 11:16:40.000000000 +0200
> +++ /usr/src/linux/kernel/mm/mmap.c	2002-10-14 11:19:32.000000000 +0200

Also, you should generate the patches without the "/usr/src/" prefix.
So it should look like this:

--- linux/mm/mmap.c.orig	2002-10-14 11:16:40.000000000 +0200
+++ linux/mm/mmap.c	2002-10-14 11:19:32.000000000 +0200

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: [PATCH] mmap.c (do_mmap_pgoff), against 2.4.19 and 2.4.20-pre10

[DervishD]

    Hi Russell :))

>        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

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

> Also, you should generate the patches without the "/usr/src/" prefix.
> So it should look like this:
> --- linux/mm/mmap.c.orig	2002-10-14 11:16:40.000000000 +0200
> +++ linux/mm/mmap.c	2002-10-14 11:19:32.000000000 +0200

    I did it on purpose, but you're right, the prefix is useless and
can cause problems. I'll resend.

    Thanks for your warning :))
    Raúl

Re: [PATCH] mmap.c (do_mmap_pgoff), against 2.4.19 and 2.4.20-pre10

[Marcelo Tosatti]

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

On Mon, 14 Oct 2002, DervishD wrote:

>     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
>

Re: [PATCH] mmap.c (do_mmap_pgoff), against 2.4.19 and 2.4.20-pre10

[DervishD]

    Hi Marcelo :)

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

    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...

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

    Well, I'll do...

> Thanks and sorry for being so unresponsive

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