Re: [Adeos-main] [PATCH] ipipe: Prevent unwritable pages after mprotect
On 2011-07-28 18:26, Jan Kiszka wrote: > On 2011-07-27 20:41, Gilles Chanteperdrix wrote: >> On 07/12/2011 09:15 AM, Jan Kiszka wrote: >>> On 2011-07-12 08:48, Gilles Chanteperdrix wrote: On 07/08/2011 02:15 PM, Jan Kiszka wrote: > On 2011-07-08 13:55, Gilles Chanteperdrix wrote: >> On 07/07/2011 09:14 PM, Jan Kiszka wrote: >>> From: Jan Kiszka >>> >>> Page protection changes issued via mprotect, e.g. to enable executable >>> stacks, cause spurious minor faults as they remove the write permission >>> from the modified range again. Avoid this by faking shared pages so that >>> vm_get_page_prot returns the desired page permissions. >> >> This looks dangerous to me. Have you checked that write to private heaps >> will not end up corrupting shared data? > > Can't follow this yet. > > If you check the comment on protection_map in mm/mmap.c, the difference > between private and shared is in real write vs. COW-able write. That's > what my patch is exploiting to get the proper arch-dependent page > protection bits. > > Are you aware of side effects or do you know a better way to inject > write permissions into the protection flags? I would tend to think that shared and private mappings do not react the same way when we write to them. >>> >>> Yes, private mappings can undergo COW. >> >> My point exactly, so, say, if the mapping is an anonymous mapping with >> only the zero page mapped, you will allow writing to the zero page, this >> does not look like a sane thing to do. The same goes for private file >> mappings where the code need to be modified (for instance relocations >> when mixing non PIC code with PIC code). > > To my understanding, and testing seems to confirm this, the branch under > vma_wants_writenotify in mprotect_fixup: if there is a need to fault ^handles this > even on shared pages (or pseudo-shared in our case), vm_page_prot will > be overwritten anyway. > Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ___ Adeos-main mailing list Adeos-main@gna.org https://mail.gna.org/listinfo/adeos-main
Re: [Adeos-main] [PATCH] ipipe: Prevent unwritable pages after mprotect
On 2011-07-27 20:41, Gilles Chanteperdrix wrote: > On 07/12/2011 09:15 AM, Jan Kiszka wrote: >> On 2011-07-12 08:48, Gilles Chanteperdrix wrote: >>> On 07/08/2011 02:15 PM, Jan Kiszka wrote: On 2011-07-08 13:55, Gilles Chanteperdrix wrote: > On 07/07/2011 09:14 PM, Jan Kiszka wrote: >> From: Jan Kiszka >> >> Page protection changes issued via mprotect, e.g. to enable executable >> stacks, cause spurious minor faults as they remove the write permission >> from the modified range again. Avoid this by faking shared pages so that >> vm_get_page_prot returns the desired page permissions. > > This looks dangerous to me. Have you checked that write to private heaps > will not end up corrupting shared data? Can't follow this yet. If you check the comment on protection_map in mm/mmap.c, the difference between private and shared is in real write vs. COW-able write. That's what my patch is exploiting to get the proper arch-dependent page protection bits. Are you aware of side effects or do you know a better way to inject write permissions into the protection flags? >>> >>> I would tend to think that shared and private mappings do not react the >>> same way when we write to them. >> >> Yes, private mappings can undergo COW. > > My point exactly, so, say, if the mapping is an anonymous mapping with > only the zero page mapped, you will allow writing to the zero page, this > does not look like a sane thing to do. The same goes for private file > mappings where the code need to be modified (for instance relocations > when mixing non PIC code with PIC code). To my understanding, and testing seems to confirm this, the branch under vma_wants_writenotify in mprotect_fixup: if there is a need to fault even on shared pages (or pseudo-shared in our case), vm_page_prot will be overwritten anyway. > >> >>> >>> In order to avoid the fault on first access, I would trigger a fault for >>> each page of the mapping, the way we do it in __rthal_arm_fault_range >>> (in ksrc/arch/arm/hal.c), but in the I-pipe kernel. >> >> That sounds like overkill, and it's potentially racy (mprotect may open >> a short window where the active PTEs do not allow writes for >> concurrently running threads). > > If a program starts writing to a mapping before mprotect returns, it is > already bogus anyway (I am talking about triggering the fault in > mprotect code of course). Unfortunately, there is also the case where the page was writable before mprotect and should remains so afterward, e.g. if you flip the execution right. So this race is real. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ___ Adeos-main mailing list Adeos-main@gna.org https://mail.gna.org/listinfo/adeos-main
Re: [Adeos-main] [PATCH] ipipe: Prevent unwritable pages after mprotect
On 07/12/2011 09:15 AM, Jan Kiszka wrote: > On 2011-07-12 08:48, Gilles Chanteperdrix wrote: >> On 07/08/2011 02:15 PM, Jan Kiszka wrote: >>> On 2011-07-08 13:55, Gilles Chanteperdrix wrote: On 07/07/2011 09:14 PM, Jan Kiszka wrote: > From: Jan Kiszka > > Page protection changes issued via mprotect, e.g. to enable executable > stacks, cause spurious minor faults as they remove the write permission > from the modified range again. Avoid this by faking shared pages so that > vm_get_page_prot returns the desired page permissions. This looks dangerous to me. Have you checked that write to private heaps will not end up corrupting shared data? >>> >>> Can't follow this yet. >>> >>> If you check the comment on protection_map in mm/mmap.c, the difference >>> between private and shared is in real write vs. COW-able write. That's >>> what my patch is exploiting to get the proper arch-dependent page >>> protection bits. >>> >>> Are you aware of side effects or do you know a better way to inject >>> write permissions into the protection flags? >> >> I would tend to think that shared and private mappings do not react the >> same way when we write to them. > > Yes, private mappings can undergo COW. My point exactly, so, say, if the mapping is an anonymous mapping with only the zero page mapped, you will allow writing to the zero page, this does not look like a sane thing to do. The same goes for private file mappings where the code need to be modified (for instance relocations when mixing non PIC code with PIC code). > >> >> In order to avoid the fault on first access, I would trigger a fault for >> each page of the mapping, the way we do it in __rthal_arm_fault_range >> (in ksrc/arch/arm/hal.c), but in the I-pipe kernel. > > That sounds like overkill, and it's potentially racy (mprotect may open > a short window where the active PTEs do not allow writes for > concurrently running threads). If a program starts writing to a mapping before mprotect returns, it is already bogus anyway (I am talking about triggering the fault in mprotect code of course). > > Jan > -- Gilles. ___ Adeos-main mailing list Adeos-main@gna.org https://mail.gna.org/listinfo/adeos-main
Re: [Adeos-main] [PATCH] ipipe: Prevent unwritable pages after mprotect
On 2011-07-12 08:48, Gilles Chanteperdrix wrote: > On 07/08/2011 02:15 PM, Jan Kiszka wrote: >> On 2011-07-08 13:55, Gilles Chanteperdrix wrote: >>> On 07/07/2011 09:14 PM, Jan Kiszka wrote: From: Jan Kiszka Page protection changes issued via mprotect, e.g. to enable executable stacks, cause spurious minor faults as they remove the write permission from the modified range again. Avoid this by faking shared pages so that vm_get_page_prot returns the desired page permissions. >>> >>> This looks dangerous to me. Have you checked that write to private heaps >>> will not end up corrupting shared data? >> >> Can't follow this yet. >> >> If you check the comment on protection_map in mm/mmap.c, the difference >> between private and shared is in real write vs. COW-able write. That's >> what my patch is exploiting to get the proper arch-dependent page >> protection bits. >> >> Are you aware of side effects or do you know a better way to inject >> write permissions into the protection flags? > > I would tend to think that shared and private mappings do not react the > same way when we write to them. Yes, private mappings can undergo COW. > > In order to avoid the fault on first access, I would trigger a fault for > each page of the mapping, the way we do it in __rthal_arm_fault_range > (in ksrc/arch/arm/hal.c), but in the I-pipe kernel. That sounds like overkill, and it's potentially racy (mprotect may open a short window where the active PTEs do not allow writes for concurrently running threads). Jan signature.asc Description: OpenPGP digital signature ___ Adeos-main mailing list Adeos-main@gna.org https://mail.gna.org/listinfo/adeos-main
Re: [Adeos-main] [PATCH] ipipe: Prevent unwritable pages after mprotect
On 07/08/2011 02:15 PM, Jan Kiszka wrote: > On 2011-07-08 13:55, Gilles Chanteperdrix wrote: >> On 07/07/2011 09:14 PM, Jan Kiszka wrote: >>> From: Jan Kiszka >>> >>> Page protection changes issued via mprotect, e.g. to enable executable >>> stacks, cause spurious minor faults as they remove the write permission >>> from the modified range again. Avoid this by faking shared pages so that >>> vm_get_page_prot returns the desired page permissions. >> >> This looks dangerous to me. Have you checked that write to private heaps >> will not end up corrupting shared data? > > Can't follow this yet. > > If you check the comment on protection_map in mm/mmap.c, the difference > between private and shared is in real write vs. COW-able write. That's > what my patch is exploiting to get the proper arch-dependent page > protection bits. > > Are you aware of side effects or do you know a better way to inject > write permissions into the protection flags? I would tend to think that shared and private mappings do not react the same way when we write to them. In order to avoid the fault on first access, I would trigger a fault for each page of the mapping, the way we do it in __rthal_arm_fault_range (in ksrc/arch/arm/hal.c), but in the I-pipe kernel. -- Gilles. ___ Adeos-main mailing list Adeos-main@gna.org https://mail.gna.org/listinfo/adeos-main
Re: [Adeos-main] [PATCH] ipipe: Prevent unwritable pages after mprotect
On 2011-07-08 13:55, Gilles Chanteperdrix wrote: > On 07/07/2011 09:14 PM, Jan Kiszka wrote: >> From: Jan Kiszka >> >> Page protection changes issued via mprotect, e.g. to enable executable >> stacks, cause spurious minor faults as they remove the write permission >> from the modified range again. Avoid this by faking shared pages so that >> vm_get_page_prot returns the desired page permissions. > > This looks dangerous to me. Have you checked that write to private heaps > will not end up corrupting shared data? Can't follow this yet. If you check the comment on protection_map in mm/mmap.c, the difference between private and shared is in real write vs. COW-able write. That's what my patch is exploiting to get the proper arch-dependent page protection bits. Are you aware of side effects or do you know a better way to inject write permissions into the protection flags? Jan signature.asc Description: OpenPGP digital signature ___ Adeos-main mailing list Adeos-main@gna.org https://mail.gna.org/listinfo/adeos-main
Re: [Adeos-main] [PATCH] ipipe: Prevent unwritable pages after mprotect
On 07/07/2011 09:14 PM, Jan Kiszka wrote: > From: Jan Kiszka > > Page protection changes issued via mprotect, e.g. to enable executable > stacks, cause spurious minor faults as they remove the write permission > from the modified range again. Avoid this by faking shared pages so that > vm_get_page_prot returns the desired page permissions. This looks dangerous to me. Have you checked that write to private heaps will not end up corrupting shared data? -- Gilles. ___ Adeos-main mailing list Adeos-main@gna.org https://mail.gna.org/listinfo/adeos-main
