Message ID | 1325605408-16401-1-git-send-email-lxnay@sabayon.org |
---|---|
State | New |
Headers | show |
ts->bprm->argv seems NULL here. Isn't it supposed to be set?
On 03.01.2012, at 17:07, Fabio Erculiani wrote: > ts->bprm->argv seems NULL here. > Isn't it supposed to be set? Good question. Maybe we need some other way to fetch argv0 then? Alex
On Tue, Jan 3, 2012 at 6:41 PM, Alexander Graf <agraf@suse.de> wrote: > > On 03.01.2012, at 17:07, Fabio Erculiani wrote: > >> ts->bprm->argv seems NULL here. >> Isn't it supposed to be set? > > Good question. Maybe we need some other way to fetch argv0 then? or we could leave just an empty string for now -> "()" on the second field of self/state. That seems to work with ps as well. Considering that the whole file doesn't bring any useful info other than the stack base and pid, we might just wait the next round of segfaults ;-) > > > Alex >
Or just using linux_binprm->filename with basename()
On 03.01.2012, at 19:08, Fabio Erculiani wrote:
> Or just using linux_binprm->filename with basename()
No, that'd be wrong since argv[0] can be different from the actual file name. Also it can be the full path or not depending on what the initiator defined.
Alex
On 03.01.2012, at 19:04, Fabio Erculiani wrote: > On Tue, Jan 3, 2012 at 6:41 PM, Alexander Graf <agraf@suse.de> wrote: >> >> On 03.01.2012, at 17:07, Fabio Erculiani wrote: >> >>> ts->bprm->argv seems NULL here. >>> Isn't it supposed to be set? >> >> Good question. Maybe we need some other way to fetch argv0 then? > > or we could leave just an empty string for now -> "()" on the second > field of self/state. > That seems to work with ps as well. > Considering that the whole file doesn't bring any useful info other > than the stack base and pid, we might just wait the next round of > segfaults ;-) Hrm. For the sake of simplicity I'd just make target_argv in main.c a global and directly access it. We only ever execute a single process in linux-user at a given time anyway. Alex
How about setting ts->bprm->argv = target_argv; ? I'm not a qemu codebase expert, but if it's always NULL (why is it NULL?) or can be NULL... It looks like can be done easily from main.c... without making a variable global.
Mumble, that is what happens already... Let me see why I get NULL here...
On 03.01.2012, at 19:46, Fabio Erculiani wrote: > How about setting ts->bprm->argv = target_argv; ? > I'm not a qemu codebase expert, but if it's always NULL (why is it > NULL?) or can be NULL... It should already be set it loader_exec. I don't know why it's NULL there for you. I suppose some debug printfs could help solve this riddle? ;) Alex
Yeah, debugging. Moreover we have this scenario: $ /bin/cat /proc/self/stat 32297 (cat) ...... I guess we should use basename() anyway...?
On 03.01.2012, at 19:54, Fabio Erculiani wrote: > Yeah, debugging. > > Moreover we have this scenario: > > $ /bin/cat /proc/self/stat > 32297 (cat) ...... > > I guess we should use basename() anyway...? argv[0] can be an arbitrary value passed in through execve. In qemu's linux-user emulation you can even override it manually with the -argv0 command line option. So no, it shouldn't be basename of the file name. Alex
Ok, I've found the reason, i guess it's a bug. target_argv pointer is placed in bprm->argv; But then target_argv is freed and nullified. loader_exec should just allocate a new char** and copy target_argv. I tried that and it worked. The problem is, where do I free() it? Am i supposed to do it or the TaskState lifecycle matches the executable (so there is no need to free() it) ?
On 03.01.2012, at 20:11, Fabio Erculiani wrote: > Ok, I've found the reason, i guess it's a bug. > target_argv pointer is placed in bprm->argv; > But then target_argv is freed and nullified. > > loader_exec should just allocate a new char** and copy target_argv. > I tried that and it worked. > > The problem is, where do I free() it? Am i supposed to do it or the > TaskState lifecycle matches the executable (so there is no need to > free() it) ? Can't you just remove the first free()? Alex
Done, it all works now ;-) !
diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 9ba51bf..f2af5d5 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -4678,14 +4678,24 @@ static int open_self_stat(void *cpu_env, int fd) int len; uint64_t val = 0; - if (i == 27) { - /* stack bottom */ - val = start_stack; + if (i == 0) { + /* pid */ + snprintf(buf, sizeof(buf), "%"PRId64 " ", getpid()); + } else if (i == 1) { + /* app name */ + snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]); + } else if (i == 27) { + /* stack bottom */ + val = start_stack; + snprintf(buf, sizeof(buf), "%"PRId64 " ", val); + } else { + /* for the rest, there is MasterCard */ + snprintf(buf, sizeof(buf), "0%c", i == 43 ? '\n' : ' '); } - snprintf(buf, sizeof(buf), "%"PRId64 "%c", val, i == 43 ? '\n' : ' '); + len = strlen(buf); if (write(fd, buf, len) != len) { - return -1; + return -1; } }
With the current fake /proc/self/stat implementation `ps` is segfaulting because it expects to read PID and argv[0] as first and second field respectively, with the latter being enclosed between backets. Reproducing is as easy as running: `ps` inside qemu-user chroot with /proc mounted. Signed-off-by: Fabio Erculiani <lxnay@sabayon.org> --- linux-user/syscall.c | 20 +++++++++++++++----- 1 files changed, 15 insertions(+), 5 deletions(-)