[FFmpeg-cvslog] r29509 - trunk/libswscale/swscale.c

[Reimar Döffinger]

On Fri, Aug 14, 2009 at 02:01:12AM +0100, M?ns Rullg?rd wrote:
> Ramiro Polla <ramiro.polla at gmail.com> writes:
> 
> > On Thu, Aug 13, 2009 at 4:24 PM, Reimar
> > D?ffinger<Reimar.Doeffinger at gmx.de> wrote:
> >> On Thu, Aug 13, 2009 at 08:29:54PM +0200, ramiro wrote:
> >>> +#ifdef MAP_ANONYMOUS
> >>> + ? ? ? ? ? ?mprotect(c->lumMmx2FilterCode, MAX_MMX2_FILTER_CODE_SIZE, PROT_EXEC | PROT_READ);
> >>> + ? ? ? ? ? ?mprotect(c->chrMmx2FilterCode, MAX_MMX2_FILTER_CODE_SIZE, PROT_EXEC | PROT_READ);
> >>
> >> Both for mmap and mprotect: Why PROT_READ though? I don't think
> >> EXEC without read is possible on most architectures, but still we
> >> do not need read access in any case I think...
> >
> > Mans, or anyone else that knows an extensive amount of platforms, can
> > you comment on this?
> 
> On ARM read permission is required to execute.  I see no harm in
> always requesting both.

mmap/mprotect can and will automatically "upgrade" the permissions.
I just see no point in requesting permissions we don't need...
About adding a mmap failure check: probably a good idea, but that is different,
if it fails you will notice by the crash anyway (and it is more work
because you'd have to think of something else to do if it fails -
well in theory for mprotect maybe too, but there is a chance it will
still work even if mprotect failed...).