2002-06-03 19:04:49

by Pavel Machek

[permalink] [raw]
Subject: Re: do_mmap

Hi!

> what about the do_mmap/do_mmap_pgoff implementation?
> reurn-type: _unsigned_ long (which should be ok cause we've to return
> an adress if len == 0)
> on error: -ERR_*
>
> and the checks in various places are really strange. - well some
> places check for:
> o != NULL
> o > -1024UL
> o ...
>
> guess this nedds some cleanup.

While you are at it... fs/binfmt_elf does mmaps but does not check for errors.
And errors actually do happen there :-(

--
Philips Velo 1: 1"x4"x8", 300gram, 60, 12MB, 40bogomips, linux, mutt,
details at http://atrey.karlin.mff.cuni.cz/~pavel/velo/index.html.


2002-06-03 19:14:02

by Thomas 'Dent' Mirlacher

[permalink] [raw]
Subject: Re: do_mmap

pavel,

> > what about the do_mmap/do_mmap_pgoff implementation?
> > reurn-type: _unsigned_ long (which should be ok cause we've to return
> > an adress if len == 0)
> > on error: -ERR_*
> >
> > and the checks in various places are really strange. - well some
> > places check for:
> > o != NULL
> > o > -1024UL
> > o ...
> >
> > guess this nedds some cleanup.
>
> While you are at it... fs/binfmt_elf does mmaps but does not check for errors.
> And errors actually do happen there :-(

sure, was tripping over that anyways ... since the compiler spits out tons
of "conversion without a cast" warnings (seems to be a different way to
use grep :)

i just came across one problem: when converting the unsigned longs to
void * pointers, everything works fine, beside the "advanced"
pointer arithmetic. (like aligning the address to a certain
boundary - using address & MASK) ... for that case i need to
cast to an unsigned long in any case :(
is there another way to do it without explicit casting?
should we introduce ptr_addr_t?
can we be sure that an unsigned long is capable to hold
and address (also in the future)
can we be sure about gcc void * pointer arithmetic (like
++ incremets the address by one)

tm

--
in some way i do, and in some way i don't.

2002-06-03 20:11:35

by Thomas 'Dent' Mirlacher

[permalink] [raw]
Subject: Re: do_mmap

pavel,

--snip/snip

> While you are at it... fs/binfmt_elf does mmaps but does not check for errors.
> And errors actually do happen there :-(

ok, had a second look.
it does check for errors, using BAD_ADDR which is: if (addr > TASK_SIZE)
and TASK_SIZE on the other hand is PAGE_OFFSET == 0xC0000000 (for x86)

hmm, which means even if we correctly map an area > PAGE_OFFSET, we will
never free the area (well another question is, if we ever try to map
an area > PAGE_OFFSET) - but nevertheless, the check should be before
trying to mmap, and checking for IS_ERR after mmap.

nyone explain what to do if we cannot map the page for MMAP_PAGE_ZERO?
- we just ignore it for now ...

also another instance: if (addr != req_address) - which means if we have
some alignment with mmap - load_elf_library just bails out.
guess we need to munmap also (if no error occured during mmap)

do_brk() is _never_ checked for return (at least in binfm_elf) ... oh
well ...

any comments?

tm

--
in some way i do, and in some way i don't.

2002-06-03 20:25:12

by Pavel Machek

[permalink] [raw]
Subject: Re: do_mmap

Hi!

> do_brk() is _never_ checked for return (at least in binfm_elf) ... oh
> well ...

That's the bug I had in my mind. It actually bitten me.

Pavel
--
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.