Re: [RFC] m68k/mm - adjust VM area to be unmapped by gap size for __iounmap()

2018-05-24 Thread Michael Schmitz 
Hi Geert,

Am 24.05.2018 um 22:02 schrieb Geert Uytterhoeven:
>> I happen to have a few old kernel trees around that I can consult for
>> a rough history, and 2.4.30 still has the comment while 2.6.18 or
>> 2.6.19 lost it.
> 
> Of course all historical git trees contain linear history, not the branch
> with the continued 2.4.x development after 2.5.x was spun off...
> 
> So I guess this was fixed in 2.4.x, but never in 2.5.x :-(

Propagating fixes to the other versions was nowhere near as automatic in
those days.

> Linux/m68k CVS had a fix in m68k-2_4_21 and later:
> 
> commit 7bfc690086f568f440f81c2bb3578dd07295b303
> Author: Geert Uytterhoeven 
> Date:   Mon Jul 21 21:36:10 2003 +
> 
> Take the gap into account in free_io_area() (from Michael Müller)
> 
...

Yep, that's it.

> I can't seem to find a public git tree with full 2.4.x history, but
> https://git.linux-mips.org/cgit/ralf/linux.git/commit/arch/m68k/mm/kmap.c?h=linux-2.4=65456e0e26a40f4c4933d700847ed1a9d261811e
> shows it made upstream in 2.4.23-rc1.

Thanks - that shows the bug was introduced in 2.2.1 and went unnoticed
until 2.4.21.

>> Where is the old CVS hosted these days?
> 
> Nowhere.
> 
> Ralf Bächle was so kind to provide me with a git import a long time ago
> (his machine was more powerful than mine ;-).
> 
> I could push it to kernel.org (a separate repo, else linux-m68k.git will grow 
> a
> lot, and everyone will suffer ;-). I also have to re-add the various branches
> (it has tags only). Anyone interested?

Might be good to have, but probably not worth the effort adding the
missing branches. I'll just bookmark Ralf's tree for now.

> Anyway, applied and queued for 4.18, with a cc to stable.
> Thanks!

Thanks, I trust Ben will pick this up for Debian once it hits stable ...

Cheers,

Michael

> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] m68k/mm - adjust VM area to be unmapped by gap size for __iounmap()

2018-05-24 Thread Geert Uytterhoeven 
Hi Michael,

On Wed, May 23, 2018 at 11:39 PM, Michael Schmitz  wrote:
> On Wed, May 23, 2018 at 7:44 PM, Geert Uytterhoeven
>  wrote:
 At first sight (and looking in full-history-linux git history), I see no
 reason for the gap. I'd assume having a block with address and size aligned
 to 256 KiB (which the caller already takes care of: IO_SIZE is 256 KiB if
 020/030 support is enabled) should be sufficient to use early termination
 tables.
>>>
>>> My guess is that someone wanted to catch out of bounds accesses by
>>> leaving the unmapped areas in between ioremapped 256 kB chunks. The
>>> unmapped gaps must 256 kB as well to avoid disturbing the alignment.
>>>
>>> The adjustment for gap size was dropped sometime between 2.4.30 and
>>> 2.6.18. At the same time, a comment in get_io_area was removed that
>>> stated 'leave a gap of IO_SIZE'. I haven't looked at the full history to
>>> find out who removed that comment (and the adjustment).
>>
>> I can't seem to find the addition/removal of that comment (checked
>> full-history-linux and the old m68k-linux CVS)?
>
> Part of the problem may be that kmap.c was moved a few times during
> integration of the nommu port.
>
> I happen to have a few old kernel trees around that I can consult for
> a rough history, and 2.4.30 still has the comment while 2.6.18 or
> 2.6.19 lost it.

Of course all historical git trees contain linear history, not the branch
with the continued 2.4.x development after 2.5.x was spun off...

So I guess this was fixed in 2.4.x, but never in 2.5.x :-(

Linux/m68k CVS had a fix in m68k-2_4_21 and later:

commit 7bfc690086f568f440f81c2bb3578dd07295b303
Author: Geert Uytterhoeven 
Date:   Mon Jul 21 21:36:10 2003 +

Take the gap into account in free_io_area() (from Michael Müller)

diff --git a/arch/m68k/mm/kmap.c b/arch/m68k/mm/kmap.c
index da23f36ec6a5..4a9676373a5b 100644
--- a/arch/m68k/mm/kmap.c
+++ b/arch/m68k/mm/kmap.c
@@ -71,7 +71,7 @@ static struct vm_struct *get_io_area(unsigned long size)
addr = tmp->size + (unsigned long)tmp->addr;
}
area->addr = (void *)addr;
-   area->size = size + IO_SIZE;
+   area->size = size + IO_SIZE;/* leave a gap between */
area->next = *p;
*p = area;
return area;
@@ -87,7 +87,10 @@ static inline void free_io_area(void *addr)
for (p =  ; (tmp = *p) ; p = >next) {
if (tmp->addr == addr) {
*p = tmp->next;
-   __iounmap(tmp->addr, tmp->size);
+   if ( tmp->size > IO_SIZE )
+   __iounmap(tmp->addr, tmp->size - IO_SIZE);
+   else
+   printk("free_io_area: Invalid I/O area
size %lu\n", tmp->size);
kfree(tmp);
return;
}

I can't seem to find a public git tree with full 2.4.x history, but
https://git.linux-mips.org/cgit/ralf/linux.git/commit/arch/m68k/mm/kmap.c?h=linux-2.4=65456e0e26a40f4c4933d700847ed1a9d261811e
shows it made upstream in 2.4.23-rc1.

> Where is the old CVS hosted these days?

Nowhere.

Ralf Bächle was so kind to provide me with a git import a long time ago
(his machine was more powerful than mine ;-).

I could push it to kernel.org (a separate repo, else linux-m68k.git will grow a
lot, and everyone will suffer ;-). I also have to re-add the various branches
(it has tags only). Anyone interested?


Anyway, applied and queued for 4.18, with a cc to stable.
Thanks!

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html