bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: fakeroot-hurd bug or not?


From: Samuel Thibault
Subject: Re: fakeroot-hurd bug or not?
Date: Wed, 23 Sep 2015 19:08:15 +0200
User-agent: Mutt/1.5.21+34 (58baf7c9f32f) (2010-12-30)

Svante Signell, le Wed 23 Sep 2015 15:45:13 +0200, a écrit :
> This test fails with fakeroot-hurd on Hurd due to that /non-existant is
> writable for faked nodes according to:

No.  Check what happens in the testcase again.  It fails due to /
being announced as writable by fakeroot (see the recursive call to
check_directory in the ENOENT case).

> running it as real root of course returns *is_writable=1 and errno="No
> such file or directory" on both Linux and Hurd.

Take care that the errno value should not be looked at when the function
succeeded. It may be just a leftover from a previous call, or even
something else, it's really undefined.

> Maybe even the tests should not be run under fakeroot at all as is
> done now with fakeroot debian/rules binary?

That is very questionable, yes.  Checks should be done in the build
target, not in the binary target.  Running tests in fakeroot does not
make much sense to me.

All that being said, we should probably not let the programs inside
fakeroot believe they can write to / (because they may then try to,
while they can't actually).

> The attached patch makes the behaviour the same as on Linux and
> fakeroot-tcp. The question is which behaviour is the expected one.

It is indeed tempting do do this change.

> Index: hurd-0.6.git20150704/trans/fakeroot.c
> ===================================================================
> --- hurd-0.6.git20150704.orig/trans/fakeroot.c
> +++ hurd-0.6.git20150704/trans/fakeroot.c
> @@ -785,11 +785,7 @@ error_t
>  netfs_report_access (struct iouser *cred, struct node *np, int *types)
>  {
>    struct netnode *nn = netfs_node_netnode (np);
> -  if (!(nn->faked & FAKE_MODE))
> -    return file_check_access (nn->file, types);
> -  else
> -    *types = O_RDWR|O_EXEC;
> -  return 0;
> +  return = file_check_access (nn->file, types);
>  }
>  
>  error_t

I'm almost believing it's the right thing to do, I'm just afraid of
corner cases.  What reassures me is that in both attempt_mkfile and
attempt_chmod, we always add RUSR and WUSR permissions on the real
node (and XUSR when it makes sense), and so for files created inside
the fakeroot session (i.e. those which have FAKE_MODE, for which we
do change the code here), the behavior will be the same in the end.
The only exception is /, which does have FAKE_MODE set in main(), but
already exists and shouldn't be made writable.  AIUI, this is exactly
where your proposed change actually change a behavior, and is precisely
what you intended to do.

Could somebody else double-check my reasoning is correct?
(we have to be careful with these permissions, we need to converge to
something that just does has the right behavior in all cases)

Perhaps for some reason it'd even fix the issues I reported on Mon, 7
Sep 2015 22:03:43 +0200, for instance the gpsd experimental package,
Svante, could you check?

Samuel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]