* [9fans] segattach off-by-one?
@ 2009-09-25 3:33 Venkatesh Srinivas
0 siblings, 0 replies; 5+ messages in thread
From: Venkatesh Srinivas @ 2009-09-25 3:33 UTC (permalink / raw)
To: Fans of the OS Plan 9 from Bell Labs
Hi,
This little program:
#include <u.h>
#include <libc.h>
#define SEGBASE ((char *) 0x10001001)
#define SEGSIZE 0x1000
void main(void) {
segattach(0, "shared", SEGBASE, SEGSIZE);
// Works fine (writing to 0x10001fff)
*(char *) (SEGBASE + SEGSIZE - 2) = 'a';
// Suicide! (writing to 0x10002000)
*(char *) (SEGBASE + SEGSIZE - 1) = 'a';
}
However, segattach's manpage claims: "... and va+len is rounded up."
Shouldn't the second page here be mapped?
I propose this patch to /sys/src/9/port/segment.c::
--- segment.c.orig 2009-09-24 22:41:59.000000000 -0400
+++ segment.c 2009-09-24 22:38:25.000000000 -0400
@@ -641,6 +641,11 @@
int sno;
Segment *s, *os;
Physseg *ps;
+ ulong ova;
+
+ ova = va;
+ va = va&~(BY2PG-1);
+ len += (ova - va);
if(va != 0 && va >= USTKTOP)
error(Ebadarg);
This patch also prevents segattaching to the zero page, which I think
was worth doing...
Could people try this? Comments?
Thanks,
-- vs
^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <<f75780240909242033k11fb23a0o237f7dbedf9a7bee@mail.gmail.com>]
* Re: [9fans] segattach off-by-one?
[not found] <<f75780240909242033k11fb23a0o237f7dbedf9a7bee@mail.gmail.com>
@ 2009-09-25 4:34 ` erik quanstrom
2009-09-25 18:01 ` Venkatesh Srinivas
0 siblings, 1 reply; 5+ messages in thread
From: erik quanstrom @ 2009-09-25 4:34 UTC (permalink / raw)
To: 9fans
(aside: "sig segattach" fails because nroff can elide
all leading space. /n/sources/patch/signit
one could argue for replacing the \*? in
selecting files with \** but no functions
return a **.)
i think segattach is already correct. consider this
#include <u.h>
#include <libc.h>
enum {
Sz = 0x1000,
};
void
main(void)
{
uchar *base, *rb;
base = (uchar*)0x10001001;
rb = segattach(0, "shared", base, Sz - 10);
print("%p\n", rb);
base[0] = 'a';
base[-1] = 'a';
base[Sz - 2] = 'a'; /* note this does not fault, even though Sz - 10 was used */
>> base[Sz - 1] = 'a';
exits("");
}
% 8.out
10001000 # not 10001001.
81 8.out: checked 8 page table entries
8.out 81: suicide: sys: trap: fault write addr=0x10002000 pc=0x1073
if i were to have used rb and not base, this base[-1] would have
faulted but base[Sz - 1] would have been okay. so i woud think
this is a misunderstanding of how segattach works.
if one wishes to disallow mapping the zero page this
condition should be changed:
if(va != 0 && va >= USTKTOP)
error(Ebadarg);
to
if(va < BY2PG || va >= USTKTOP)
- erik
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [9fans] segattach off-by-one?
2009-09-25 4:34 ` erik quanstrom
@ 2009-09-25 18:01 ` Venkatesh Srinivas
2009-09-25 18:18 ` erik quanstrom
0 siblings, 1 reply; 5+ messages in thread
From: Venkatesh Srinivas @ 2009-09-25 18:01 UTC (permalink / raw)
To: Fans of the OS Plan 9 from Bell Labs
So in my example, va = 0x10001001, len = 0x1000. I understood that to
mean [0x10001001, 0x10002001) was the newly-valid interval, which
would mean 0x10002000 was a valid address...
The segattach manpage says va+len is 'rounded up'; wouldn't that mean
the expanded interval was [0x10001000, 0x10003000)? Or am I misreading
the manpage?
Thanks,
-- vs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [9fans] segattach off-by-one?
2009-09-25 18:01 ` Venkatesh Srinivas
@ 2009-09-25 18:18 ` erik quanstrom
2009-09-25 18:32 ` erik quanstrom
0 siblings, 1 reply; 5+ messages in thread
From: erik quanstrom @ 2009-09-25 18:18 UTC (permalink / raw)
To: 9fans
On Fri Sep 25 14:11:03 EDT 2009, me@acm.jhu.edu wrote:
> So in my example, va = 0x10001001, len = 0x1000. I understood that to
> mean [0x10001001, 0x10002001) was the newly-valid interval, which
> would mean 0x10002000 was a valid address...
i think you're misreading the man page. from the man page
Segattach creates a new memory segment, adds it to the call-
ing process's address space, and returns its lowest address.
so you need to use the value returned, not the value
you gave it. in this case 0x10001000 not 0x0001001.
also from the man page
Va and len specify the position of the segment in the
process's address space. Va is rounded down to the nearest
page boundary and va+len is rounded up. The system does not
so va is rounded down (as you see) and the length
is rounded up, also as you see.
- erik
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [9fans] segattach off-by-one?
2009-09-25 18:18 ` erik quanstrom
@ 2009-09-25 18:32 ` erik quanstrom
0 siblings, 0 replies; 5+ messages in thread
From: erik quanstrom @ 2009-09-25 18:32 UTC (permalink / raw)
To: 9fans
the patch i posted yesterday was wrong. i forgot about
automatic address assigment with segattach. this works,
but perhaps it would be better to round va before checking
if va == 0?
/n/quanstro//sys/src/9/port/segment.c:642,648 - segment.c:642,648
Segment *s, *os;
Physseg *ps;
- if((va !=0 && va < BY2PG) || va >= USTKTOP)
+ if(va != 0 && va >= USTKTOP)
error(Ebadarg);
validaddr((ulong)name, 1, 0);
- erik
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-09-25 18:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-25 3:33 [9fans] segattach off-by-one? Venkatesh Srinivas
[not found] <<f75780240909242033k11fb23a0o237f7dbedf9a7bee@mail.gmail.com>
2009-09-25 4:34 ` erik quanstrom
2009-09-25 18:01 ` Venkatesh Srinivas
2009-09-25 18:18 ` erik quanstrom
2009-09-25 18:32 ` erik quanstrom
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).