cpio is busted

[Barret Rhoden]

If you put a cpio in /mnt, say obj/kern/initramfs.cpio, then try to
extract it:

ash ifconfig
ash root
cd dir1 (so as to not run into errors where files already exist)
cpio -d -i < /mnt/initramfs.cpio

You get a stat error.

This worked on

ddcb5d3bf651 ("Replaced dummy likely/unlikely definitions with
the ones coming from compiler.h.")

but fails on origin/master.


As an aside, git bisect had a hard time too. Commit

4e006e59f626 ("Avoid an extra function call on the error frame
handling.")

won't build, due to having a set_error() function (which does not exist
yet in the git repo). that's probably a rebase problem.

I'll hack up the bisect a bit to get it to work, but this is an example
of why I like having a clean history.

Barret

[Barret Rhoden]

On 2015-10-14 at 18:25 Barret Rhoden <br...@cs.berkeley.edu> wrote:
> If you put a cpio in /mnt, say obj/kern/initramfs.cpio, then try to
> extract it:
>
> ash ifconfig
> ash root
> cd dir1 (so as to not run into errors where files already exist)
> cpio -d -i < /mnt/initramfs.cpio
>
> You get a stat error.
>
> This worked on
>
> ddcb5d3bf651 ("Replaced dummy likely/unlikely definitions with
> the ones coming from compiler.h.")
>
> but fails on origin/master.

On a related note, ac4ef831b966 ("Added explicit errno reporting from
error() API.") page faults at boot.

The issue was that error(x, NULL) in devwalk() triggers a page fault,
since the vprintf code isn't expecting a null pointer. It gets fixed
in d1783c877cce ("Dropped char* error file to unify common error
strings handling.")

Barret

[Davide Libenzi]

Good point about clean history. Bisect breaks badly if not.

All my commits are typically either building fine, or gets squashed into one building fine.

Did that escape?

Barret

--
You received this message because you are subscribed to the Google Groups "Akaros" group.
To unsubscribe from this group and stop receiving emails from it, send an email to akaros+un...@googlegroups.com.
To post to this group, send email to aka...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Barret Rhoden]

First bad commit: ac4ef831b966 ("Added explicit errno reporting from
error() API.")

My guess is some error code was treated specially, but now it is a new
value that conflicts with its prior special treatment.

If someone figures it out or makes progress on it, let me know. o/w
I'll take a look tomorrow.

Barret

[Davide Libenzi]

I will take a look.

/methink the NULL pointer is an effect of having moved around commits during the WIP works.

Barret

[Barret Rhoden]

Cool, thanks.

On 2015-10-14 at 15:52 "'Davide Libenzi' via Akaros"

[Davide Libenzi]

Did not have time to dig into it, by trying to replicate yet.

Glancing at the code, in sysfile.c, there is something which needs fixing, though, I am not sure that is the culprit.

We do like:

int fd = somefunc(...); // somefunc returns >= 0 if OK, or -Esomething in case of error

if (fd < 0)

    error(fd, "blah ...");

Should be error(-fd, ...).

What exact error do you get from cpio?

[Barret Rhoden]

On 2015-10-14 at 20:41 "'Davide Libenzi' via Akaros"

<aka...@googlegroups.com> wrote:
> Did not have time to dig into it, by trying to replicate yet.
> Glancing at the code, in sysfile.c, there is something which needs
> fixing, though, I am not sure that is the culprit.
> We do like:
>
> int fd = somefunc(...); // somefunc returns >= 0 if OK, or
> -Esomething in case of error
>
> if (fd < 0)
> error(fd, "blah ...");
>
> Should be error(-fd, ...).

Yeah, that seems a little weird. There might be other cases floating
around where errno is set by a function (now with set_errno()), and
then we come along later and want to do an error() without changing
errno.

> What exact error do you get from cpio?

/ $ ash ifconfig
Welcome to QEMU!
/etc/network/local not found
bind #cons -> /dev flag 1
bind #proc -> /proc flag 1
bind #srv -> /srv flag 1
bind #ip -> /net flag 2
bind #ether.0 -> /net flag 2
I am 10.0.2.15, default route 10.0.2.2
cs hasn't created #srv/cs yet, sleeping until it does...
bind #kprof -> /prof/ flag 2
ifconfig complete
/ $ ash root
bin hello-go somefile
go_get_html initramfs.cpio
/ $ cd dir1
/dir1/ $ cpio -d -i < /mnt/init
/dir1/ $ cpio -d -i < /mnt/initramfs.cpio
cpio: can't stat old file: No such device, No such device

The repeating of "No such device" twice is due to using
errno_to_errstring in error(), such that we're saying ENODEV, "No such
device". Userspace doesn't know that errstr is a copy of the text of
ENODEV, so it does its own strerror, followed by a verbatim dump of
errstr.

The "can't stat old file" comes from busybox itself.

Barret

[Davide Libenzi]

Thanks, started looking into it ...

[Davide Libenzi]

Does it do it with every cpio archive? I tried a simple one and it does not.

Can I have yours (both)?

[Barret Rhoden]

On 2015-10-15 at 07:53 "'Davide Libenzi' via Akaros"

<aka...@googlegroups.com> wrote:
> Does it do it with every cpio archive? I tried a simple one and it
> does not. Can I have yours (both)?

It does it for me with obj/kern/initramfs.cpio, but only if you cd into
dir1 first. I'll send you a cpio offline, since it's a little large for
email.

barret

[Davide Libenzi]

Oh, wait, you are likely using the one generated by the build, right?

Let me try that ...

[Barret Rhoden]

So busybox is doing this:

/* Remove the existing entry if its older than the extracted entry */
struct stat existing_sb;
if (lstat(file_header->name, &existing_sb) == -1) {
if (errno != ENOENT) {
bb_perror_msg_and_die("can't stat old file");
}
}

From putting some printks in syscall.c (stat_helper()), we're getting
stuff like this:

STAT HELPER FOR usr/
retval 0, errno 0
STAT HELPER FOR usr/some/crap/to/extract
retval -1, errno 19

The file doesn't exist, but we're getting ENODEV (19) instead of ENOENT.

There's a couple places in ac4ef831b966 ("Added explicit errno
reporting from error() API.") that do stuff like

@@ -427,17 +427,17 @@ int netifwstat(struct ether *nif, struct chan *c, uint8_t * db, int n)
f = nif->f[NETID(c->qid.path)];
if (f == 0) {
set_errno(ENOENT);
- error(Enonexist);
+ error(ENODEV, NULL);

That error() call replaces ENOENT with ENODEV, which messes up
userspace.

I'll try fixing those up and see if it helps.

Barret



On 2015-10-15 at 08:02 "'Davide Libenzi' via Akaros"

[Davide Libenzi]

This is what I get:

/dir1/ $ cpio -d -i < ../initramfs.cpio 
cpio: can't remove old file root: Operation not permitted, Operation not permitted

It seems that cpio does not care in which directory I am, and always unpack into /?

The Linux version has a flag for -no-absolute-filenames

[Davide Libenzi]

BTW, I have already a branch when I fixed the error(-fd, ..) stuff.

If you want I can fix them myself.

[Barret Rhoden]

On 2015-10-15 at 08:15 "'Davide Libenzi' via Akaros"

<aka...@googlegroups.com> wrote:
> This is what I get:
>
> /dir1/ $ cpio -d -i < ../initramfs.cpio
> cpio: can't remove old file root: Operation not permitted, Operation
> not permitted
>
>
> It seems that cpio does not care in which directory I am, and always
> unpack into /?

Mine seemed to care, since if I tried to extract into /, I'd get a
bunch of "file already exists" errors. The busybox error case was when
it was trying to extract something that didn't exist yet.

stand by, i think i have a fix.

barret

[Kevin Klues]

Resolved by: https://groups.google.com/forum/#!topic/akaros/fQ71jmsXQPo