Debian Bug report logs - #190732
hurd: non-priviledged user may crash filesystem

version graph

Package: hurd; Maintainer for hurd is GNU Hurd Maintainers <debian-hurd@lists.debian.org>; Source for hurd is src:hurd.

Reported by: Robert Millan <rmh@debian.org>

Date: Fri, 25 Apr 2003 12:18:01 UTC

Severity: critical

Tags: fixed-upstream, patch

Found in version 20021118-2

Done: Michael Banck <mbanck@debian.org>

Bug is archived. No further changes may be made.

Toggle useless messages

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>, hurd@packages.qa.debian.org:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Robert Millan <rmh@debian.org>:
New Bug report received and forwarded. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>, hurd@packages.qa.debian.org. Full text and rfc822 format available.

Message #5 received at submit@bugs.debian.org (full text, mbox):

From: Robert Millan <rmh@debian.org>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: hurd: non-priviledged user may crash filesystem
Date: Fri, 25 Apr 2003 14:04:47 +0200
Package: hurd
Version: 20021118-2
Severity: critical

by exploiting this bug, a non-priviledged user is able to crash
a filesystem on which he/she has read/write access to. if that
filesystem is /, then is able to crash the whole system.

test log:

$ dd if=/dev/zero of=./fs ibs=32k count=10 ; mke2fs -o hurd ./fs
[...]
$ settrans -cafg ./mnt /hurd/ext2fs ./fs
$ cat cbtf
#!/bin/sh -x
# crashes the filesystem on which it is being run.
# (caution: if that filesystem is /, crashes the system)
rm -rf no-write dir
mkdir -p no-write/dir
chmod 555 no-write
mv no-write/dir .
$ ./cbtf
+ rm -rf no-write dir
+ mkdir -p no-write/dir
+ chmod 555 no-write
+ mv no-write/dir .
ext2fs: ../../libdiskfs/dir_renamed.c: 202: diskfs_rename_dir: Assertion `tmpnp = fnp' failed.
mv: cannot move `no_write/dir' to `./dir': Computer bought the farm

-- System Information:
Debian Release: testing/unstable
Architecture: hurd-i386
Kernel: GNU aragorn 0.3 GNUmach-1.2/Hurd-0.3 i386-AT386
Locale: LANG=C, LC_CTYPE=C

Versions of packages hurd depends on:
ii  libc0.3                  2.3.1-5         GNU C Library: Shared libraries an
ii  libncursesw5             5.2.20020112a-8 Shared libraries for terminal hand

-- no debconf information




Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>, hurd@packages.qa.debian.org:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Robert Millan <zeratul2@wanadoo.es>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>, hurd@packages.qa.debian.org. Full text and rfc822 format available.

Message #10 received at 190732@bugs.debian.org (full text, mbox):

From: Robert Millan <zeratul2@wanadoo.es>
To: 190732@bugs.debian.org
Subject: little note
Date: Tue, 29 Apr 2003 12:40:34 +0200
hi,
                                                                               
this is just a notification for me (and anyone interested)
to keep track on what to do when this bug is fixed:
                                                                               
        this one breaks coreutils (tests/mv/perm-1)

-- 
Robert Millan

make: *** No rule to make target `war'.  Stop.

Another world is possible - Just say no to genocide



Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Ognyan Kulev <ogi@fmi.uni-sofia.bg>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #15 received at 190732@bugs.debian.org (full text, mbox):

From: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
To: 190732@bugs.debian.org
Subject: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Wed, 11 Jun 2003 18:48:18 +0300
[Message part 1 (text/plain, inline)]
Hi,

I made a somewhat reworked version of lbidiskfs/dir-renamed.c.  This is 
the second attached patch.  As you may guess, I prefer it over the other 
one.

But there is a much smaller patch that fixes the bug too.  It is the 
first attached patch.

Regards
-- 
Ognyan Kulev <ogi@fmi.uni-sofia.bg>, "\"Programmer\""
7D9F 66E6 68B7 A62B 0FCF  EB04 80BF 3A8C A252 9782
[dir-renamed.1.changelog (text/plain, inline)]
2003-06-11  Ognyan Kulev  <ogi@fmi.uni-sofia.bg>

	* dir-renamed.c (diskfs_rename_dir): Fixed assertion. Check
	permissions to remove FROMNAME before any modification could take
	place.
[dir-renamed.1.diff (text/plain, inline)]
--- hurd/libdiskfs/dir-renamed.c.~1.22.~	2001-10-12 05:49:17.000000000 +0300
+++ hurd/libdiskfs/dir-renamed.c	2003-06-11 18:46:21.000000000 +0300
@@ -1,5 +1,5 @@
 /*
-   Copyright (C) 1994,95,96,97,98,99,2001 Free Software Foundation, Inc.
+   Copyright (C) 1994,95,96,97,98,99,2001,03 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License as
@@ -106,8 +106,15 @@
       return 0;
     }
 
-  /* Now we can safely lock fnp */
-  mutex_lock (&fnp->lock);
+  /* Check permissions to remove FROMNAME and lock FNP.  */
+  tmpds = alloca (diskfs_dirstat_size);
+  err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, tmpds, fromcred);
+  assert (!tmpnp || tmpnp == fnp);
+  if (tmpnp)
+    diskfs_nrele (tmpnp);
+  diskfs_drop_dirstat (fdp, tmpds);
+  if (err)
+    goto out;
 
   if (tnp)
     {
@@ -199,8 +206,9 @@
   ds = buf;
   mutex_unlock (&fnp->lock);
   err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, ds, fromcred);
-  assert (tmpnp == fnp);
-  diskfs_nrele (tmpnp);
+  assert (!tmpnp || tmpnp == fnp);
+  if (tmpnp)
+    diskfs_nrele (tmpnp);
   if (err)
     goto out;
 
[dir-renamed.2.changelog (text/plain, inline)]
2003-06-11  Ognyan Kulev  <ogi@fmi.uni-sofia.bg>

	* dir-renamed.c (diskfs_rename_dir): Reorder the statements so
	that the whole function behave as atomic as possible when errors
	occur.
	(checkpath): Removed redundant statement.

[dir-renamed.2.diff (text/plain, inline)]
--- hurd/libdiskfs/dir-renamed.c.~1.22.~	2001-10-12 05:49:17.000000000 +0300
+++ hurd/libdiskfs/dir-renamed.c	2003-06-11 21:17:25.000000000 +0300
@@ -1,5 +1,5 @@
 /*
-   Copyright (C) 1994,95,96,97,98,99,2001 Free Software Foundation, Inc.
+   Copyright (C) 1994,95,96,97,98,99,2001,03 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License as
@@ -32,9 +32,8 @@ checkpath(struct node *source,
   error_t err;
   struct node *np;
 
-  np = target;
   for (np = target, err = 0;
-       /* nothing */;
+       np != diskfs_root_node;
        /* This special lookup does a diskfs_nput on its first argument
 	  when it succeeds. */
        err = diskfs_lookup (np, "..", LOOKUP | SPEC_DOTDOT, &np, 0, cred))
@@ -50,13 +49,11 @@ checkpath(struct node *source,
 	  diskfs_nput (np);
 	  return EINVAL;
 	}
-
-      if (np == diskfs_root_node)
-	{
-	  diskfs_nput (np);
-	  return 0;
-	}
     }
+
+  /* We've reached the root node.  */
+  diskfs_nput (np);
+  return 0;
 }
 
 /* Rename directory node FNP (whose parent is FDP, and which has name
@@ -72,45 +69,43 @@ diskfs_rename_dir (struct node *fdp, str
 		   struct protid *fromcred, struct protid *tocred)
 {
   error_t err;
-  struct node *tnp, *tmpnp;
-  void *buf = alloca (diskfs_dirstat_size);
-  struct dirstat *ds;
-  struct dirstat *tmpds;
+  struct node *tnp, *fn_tmp, *fnddp;
+  struct dirstat *tn_ds = 0, *fndd_ds = 0, *fn_ds = 0;
+
+  /* 1: Prepare for modifications.  Various checks are performed so
+     that as many errors as possible are catched before any change in
+     disk nodes.  */
 
+  /* Is TDP child of FNP?  */
   mutex_lock (&tdp->lock);
   diskfs_nref (tdp);		/* reference and lock will get consumed by
 				   checkpath */
   err = checkpath (fnp, tdp, tocred);
-
   if (err)
     return err;
 
-  /* Now, lock the parent directories.  This is legal because tdp is not
-     a child of fnp (guaranteed by checkpath above). */
+  /* Now, lock the parent directories.  This is legal because TDP is not
+     a child of FNP (guaranteed by checkpath above). */
   mutex_lock (&fdp->lock);
   if (fdp != tdp)
     mutex_lock (&tdp->lock);
 
-  /* 1: Lookup target; if it exists, make sure it's an empty directory. */
-  ds = buf;
-  err = diskfs_lookup (tdp, toname, RENAME, &tnp, ds, tocred);
+  /* Get TONAME's node in TNP and its dirstat in TN_DS.  */
+  tn_ds = alloca (diskfs_dirstat_size);
+  err = diskfs_lookup (tdp, toname, RENAME, &tnp, tn_ds, tocred);
   assert (err != EAGAIN);	/* <-> assert (TONAME != "..") */
 
   if (tnp == fnp)
+    /* Source and target are the same node.  */
     {
-      diskfs_drop_dirstat (tdp, ds);
-      diskfs_nput (tnp);
-      mutex_unlock (&tdp->lock);
-      if (fdp != tdp)
-	mutex_unlock (&fdp->lock);
-      return 0;
+      fnp = 0;	/* Don't unlock FNP as it's not locked yet. */
+      err = 0;
+      goto out;
     }
 
-  /* Now we can safely lock fnp */
-  mutex_lock (&fnp->lock);
-
   if (tnp)
     {
+      /* If TONAME exists, then TNP must be empty directory.  */
       if (! S_ISDIR(tnp->dn_stat.st_mode))
 	err = ENOTDIR;
       else if (!diskfs_dirempty (tnp, tocred))
@@ -118,64 +113,89 @@ diskfs_rename_dir (struct node *fdp, str
     }
 
   if (err && err != ENOENT)
-    goto out;
+    {
+      fnp = 0;
+      goto out;
+    }
+
+  /* Get FROMNAME's dirstat in FN_DS.  Lock FNP.  */
+  fn_ds = alloca (diskfs_dirstat_size);
+  mutex_unlock (&fnp->lock);
+  err = diskfs_lookup (fdp, fromname, REMOVE, &fn_tmp, fn_ds, fromcred);
+  assert (!fn_tmp || fn_tmp == fnp);
+  if (fn_tmp)
+    diskfs_nrele (fn_tmp);
+  if (err)
+    {
+      fnp = 0;
+      goto out;
+    }
+  diskfs_drop_dirstat (fdp, fn_ds);
+  fn_ds = 0;
 
-  /* 2: Set our .. to point to the new parent */
   if (fdp != tdp)
+    /* We'll move across directories.  */
     {
-      if (tdp->dn_stat.st_nlink == diskfs_link_max - 1)
+      /* Get FNP's ".." dirstat in FNDD_DS.  */
+      fndd_ds = alloca (diskfs_dirstat_size);
+      err = diskfs_lookup (fnp, "..", RENAME | SPEC_DOTDOT,
+			   &fnddp, fndd_ds, fromcred);
+      assert (err != ENOENT);
+      if (err)
+	goto out;
+      assert (fnddp == fdp);
+
+      if (tdp->dn_stat.st_nlink == diskfs_link_max - 1
+	  || fnp->dn_stat.st_nlink == diskfs_link_max - 1)
+	/* TDP can't afford another link (by FNP) to it,
+	   or FNP can't afford another link (by TDP) to it.  */
 	{
 	  err = EMLINK;
-	  return EMLINK;
+	  goto out;
 	}
+    }
+
+  /* 2: Modify disk nodes to match the new state.  */
+
+  if (fdp != tdp)
+    /* Set FNP's .. to point to the new parent TDP.   */
+    {
+      /* TDP has new child (FNP).  */
       tdp->dn_stat.st_nlink++;
       tdp->dn_set_ctime = 1;
       if (diskfs_synchronous)
 	diskfs_node_update (tdp, 1);
 
-      tmpds = alloca (diskfs_dirstat_size);
-      err = diskfs_lookup (fnp, "..", RENAME | SPEC_DOTDOT,
-			   &tmpnp, tmpds, fromcred);
-      assert (err != ENOENT);
-      if (err)
-	{
-	  diskfs_drop_dirstat (fnp, tmpds);
-	  goto out;
-	}
-      assert (tmpnp == fdp);
-
-      err = diskfs_dirrewrite (fnp, fdp, tdp, "..", tmpds);
+      /* FNP has new parent (TDP).  */
+      err = diskfs_dirrewrite (fnp, fdp, tdp, "..", fndd_ds);
+      fndd_ds = 0;
       if (diskfs_synchronous)
 	diskfs_file_update (fnp, 1);
-      if (err)
-	goto out;
 
+      /* FNP is no longer child of FDP.  */
       fdp->dn_stat.st_nlink--;
       fdp->dn_set_ctime = 1;
       if (diskfs_synchronous)
 	diskfs_node_update (fdp, 1);
+
+      if (err)
+	goto out;
     }
 
+  /* Increment the link count on the node being moved and rewrite
+     TDP. */
 
-  /* 3: Increment the link count on the node being moved and rewrite
-     tdp. */
-  if (fnp->dn_stat.st_nlink == diskfs_link_max - 1)
-    {
-      mutex_unlock (&fnp->lock);
-      diskfs_drop_dirstat (tdp, ds);
-      mutex_unlock (&tdp->lock);
-      if (tnp)
-	diskfs_nput (tnp);
-      return EMLINK;
-    }
+  /* Add FNP to TDP.  */
+  /* XXX ogi: Why is this needed when below st_nlink is decremented?  */
   fnp->dn_stat.st_nlink++;
   fnp->dn_set_ctime = 1;
   diskfs_node_update (fnp, 1);
 
   if (tnp)
     {
-      err = diskfs_dirrewrite (tdp, tnp, fnp, toname, ds);
-      ds = 0;
+      /* Replace TNP with FNP.  */
+      err = diskfs_dirrewrite (tdp, tnp, fnp, toname, tn_ds);
+      tn_ds = 0;
       if (!err)
 	{
 	  tnp->dn_stat.st_nlink--;
@@ -187,7 +207,9 @@ diskfs_rename_dir (struct node *fdp, str
     }
   else
     {
-      err = diskfs_direnter (tdp, toname, fnp, ds, tocred);
+      /* FNP is new child of TDP.  */
+      err = diskfs_direnter (tdp, toname, fnp, tn_ds, tocred);
+      tn_ds = 0;
       if (diskfs_synchronous)
 	diskfs_file_update (tdp, 1);
     }
@@ -195,17 +217,25 @@ diskfs_rename_dir (struct node *fdp, str
   if (err)
     goto out;
 
-  /* 4: Remove the entry in fdp. */
-  ds = buf;
+  /* Get FROMNAME's dirstat in FN_DS.  XXX: This is the same code as
+     in the preparation.  The reason is that the information taken
+     before is unreliable but it serves to check permissions.  */
+  fn_ds = alloca (diskfs_dirstat_size);
   mutex_unlock (&fnp->lock);
-  err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, ds, fromcred);
-  assert (tmpnp == fnp);
-  diskfs_nrele (tmpnp);
+  err = diskfs_lookup (fdp, fromname, REMOVE, &fn_tmp, fn_ds, fromcred);
+  assert (!fn_tmp || fn_tmp == fnp);
+  if (fn_tmp)
+    diskfs_nrele (fn_tmp);
   if (err)
-    goto out;
+    {
+      fnp = 0;
+      goto out;
+    }
 
-  diskfs_dirremove (fdp, fnp, fromname, ds);
-  ds = 0;
+  /* Remove FNP from FDP. */
+  err = diskfs_dirremove (fdp, fnp, fromname, fn_ds);
+  assert (!err);
+  fn_ds = 0;
   fnp->dn_stat.st_nlink--;
   fnp->dn_set_ctime = 1;
   if (diskfs_synchronous)
@@ -214,6 +244,8 @@ diskfs_rename_dir (struct node *fdp, str
       diskfs_node_update (fnp, 1);
     }
 
+  /* 3: Cleanups when all is over or error has occurred.  */
+
  out:
   if (tdp)
     mutex_unlock (&tdp->lock);
@@ -223,7 +255,11 @@ diskfs_rename_dir (struct node *fdp, str
     mutex_unlock (&fdp->lock);
   if (fnp)
     mutex_unlock (&fnp->lock);
-  if (ds)
-    diskfs_drop_dirstat (tdp, ds);
+  if (tn_ds)
+    diskfs_drop_dirstat (tdp, tn_ds);
+  if (fn_ds)
+    diskfs_drop_dirstat (tdp, fn_ds);
+  if (fndd_ds)
+    diskfs_drop_dirstat (tdp, fndd_ds);
   return err;
 }

Tags added: patch Request was from Ognyan Kulev <ogi@fmi.uni-sofia.bg> to control@bugs.debian.org. Full text and rfc822 format available.

Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #22 received at 190732@bugs.debian.org (full text, mbox):

From: Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>
To: 190732@bugs.debian.org, Ognyan Kulev <ogi@fmi.uni-sofia.bg>
Subject: Re: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Tue, 15 Jul 2003 05:43:24 -0500
Hi,

I think the first patch is basically correct, although I wonder about
the following details:

1. What errors do you still expect from the second time diskfs_lookup is
invoked with REMOVE?  Is there any error that is allowed at that time?
If there is, the change is correct.

2. Is it possible, and according to diskfs.h it should be, to just
call dir_lookup with REMOVE only once, at the point you introduced it
in your patch, and store the dirstat until it is needed for the actual
disks_dirremove call.  This saves one lookup call.  Can you try such a
change and look into the code if it is feasible?

Thanks,
Marcus



Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Ognyan Kulev <ogi@fmi.uni-sofia.bg>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #27 received at 190732@bugs.debian.org (full text, mbox):

From: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
To: Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>
Cc: 190732@bugs.debian.org, Ognyan Kulev <ogi@fmi.uni-sofia.bg>
Subject: Re: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Wed, 16 Jul 2003 01:26:20 +0300
[Message part 1 (text/plain, inline)]
On Tue, Jul 15, 2003 at 05:43:24AM -0500, Marcus Brinkmann wrote:
> 1. What errors do you still expect from the second time diskfs_lookup is
> invoked with REMOVE?  Is there any error that is allowed at that time?
> If there is, the change is correct.

Before the second diskfs_lookup, mutex_unlock is called.  This leaves a 
little time when someone could remove the node.

> 2. Is it possible, and according to diskfs.h it should be, to just
> call dir_lookup with REMOVE only once, at the point you introduced it
> in your patch, and store the dirstat until it is needed for the actual
> disks_dirremove call.  This saves one lookup call.  Can you try such a
> change and look into the code if it is feasible?

While trying to do that now, I recalled why this cannot be done.  If
diskfs_lookup is called only once in the beginning, the assertion in 
ext2fs/dir.c:716 is triggered with the following commands:

$ mkdir x
$ mv x y
$ mv y x

It seems that dirstat of ext2fs can break when there are other 
activities in the directory.  It's possible that ext2fs is wrong, but I 
didn't look further.

I send a revisited patch that I beleive is easier to read, and it fixes 
some tiny bugs too.

Regards
--
Ognyan Kulev <ogi@fmi.uni-sofia.bg>
[dir-renamed.3.diff (text/plain, attachment)]

Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Marco Gerards <metgerards@student.han.nl>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #32 received at 190732@bugs.debian.org (full text, mbox):

From: Marco Gerards <metgerards@student.han.nl>
To: Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>
Cc: 190732@bugs.debian.org, Ognyan Kulev <ogi@fmi.uni-sofia.bg>
Subject: Re: Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: 15 Jul 2003 23:17:34 +0200
Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de> writes:

> Hi,
> 
> I think the first patch is basically correct, although I wonder about
> the following details:
> 
> 1. What errors do you still expect from the second time diskfs_lookup is
> invoked with REMOVE?  Is there any error that is allowed at that time?
> If there is, the change is correct.

I didn't see this patch yet (only the first one, which is terribly
wrong, but it was written by someone else). Is it ok if I have a look
at this patch tomorrow?

> 2. Is it possible, and according to diskfs.h it should be, to just
> call dir_lookup with REMOVE only once, at the point you introduced it
> in your patch, and store the dirstat until it is needed for the actual
> disks_dirremove call.  This saves one lookup call.  Can you try such a
> change and look into the code if it is feasible?

Funny, I wrote such patch yesterday. There is still a bug in what I
wrote, so I didn't send it in yet. Do you want me to fix it or send it
in or is saying what you proposed is possible and easy to do enough?
:)

I have a look at this patch tommorrow to check if it has the same
problem with my patch and send in a new patch or I'll say what the
problem is.

Thanks,
Marco




Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #37 received at 190732@bugs.debian.org (full text, mbox):

From: Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>
To: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
Cc: Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>, 190732@bugs.debian.org
Subject: Re: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Wed, 16 Jul 2003 05:59:20 -0500
On Wed, Jul 16, 2003 at 01:26:20AM +0300, Ognyan Kulev wrote:
> On Tue, Jul 15, 2003 at 05:43:24AM -0500, Marcus Brinkmann wrote:
> > 1. What errors do you still expect from the second time diskfs_lookup is
> > invoked with REMOVE?  Is there any error that is allowed at that time?
> > If there is, the change is correct.
> 
> Before the second diskfs_lookup, mutex_unlock is called.  This leaves a 
> little time when someone could remove the node.

The node is unlocked, but the directory is not.  You can not unlink a
file while the directory is locked.  There might be other errors to be
concerned about?
 
> > 2. Is it possible, and according to diskfs.h it should be, to just
> > call dir_lookup with REMOVE only once, at the point you introduced it
> > in your patch, and store the dirstat until it is needed for the actual
> > disks_dirremove call.  This saves one lookup call.  Can you try such a
> > change and look into the code if it is feasible?
> 
> While trying to do that now, I recalled why this cannot be done.  If
> diskfs_lookup is called only once in the beginning, the assertion in 
> ext2fs/dir.c:716 is triggered with the following commands:
> 
> $ mkdir x
> $ mv x y
> $ mv y x
> 
> It seems that dirstat of ext2fs can break when there are other 
> activities in the directory.  It's possible that ext2fs is wrong, but I 
> didn't look further.

Yeah, this seems to be in particular the case if the source and target
directory are the same?  I don't have time to really crunch all that
code until I have figured out what is safe and what is supposed to be
safe.  The easy fix for now, and likely the thing that is really
required, is dropping the dirstat as you did in your patch.
 
> I send a revisited patch that I beleive is easier to read, and it fixes 
> some tiny bugs too.

Thanks.  For now, not being in the position to do much testing and
analysis, I will approve the simple patch and leave the rewritten
version for later.

Thanks,
Marcus



Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Marco Gerards <metgerards@student.han.nl>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #42 received at 190732@bugs.debian.org (full text, mbox):

From: Marco Gerards <metgerards@student.han.nl>
To: 190732@bugs.debian.org
Cc: Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>, metgerards@student.han.nl, Ognyan Kulev <ogi@fmi.uni-sofia.bg>
Subject: Re: Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: 16 Jul 2003 20:17:28 +0200
Marco Gerards <metgerards@student.han.nl> writes:

> I have a look at this patch tommorrow to check if it has the same
> problem with my patch and send in a new patch or I'll say what the
> problem is.

The patch worked perfectly for me!

As I've promissed, I've included my version with this mail. Perhaps it
can be useful.

Thanks,
Marco

2003-07-16  Marco Gerards  <metgerards@student.han.nl>

	* dir-renamed.c (diskfs_rename_dir): Move the diskfs_lookup for
	REMOVE up to correctly check for access rights. Move the assertion
	to after the error check.

Common subdirectories: /home/marco/src/hurdcvs/hurd/libdiskfs/CVS and libdiskfs/CVS
diff -up /home/marco/src/hurdcvs/hurd/libdiskfs/dir-renamed.c libdiskfs/dir-renamed.c
--- /home/marco/src/hurdcvs/hurd/libdiskfs/dir-renamed.c	2001-10-12 04:49:17.000000000 +0200
+++ libdiskfs/dir-renamed.c	2003-07-16 21:41:57.000000000 +0200
@@ -76,6 +76,7 @@ diskfs_rename_dir (struct node *fdp, str
   void *buf = alloca (diskfs_dirstat_size);
   struct dirstat *ds;
   struct dirstat *tmpds;
+  struct dirstat *remds = 0;
 
   mutex_lock (&tdp->lock);
   diskfs_nref (tdp);		/* reference and lock will get consumed by
@@ -120,6 +121,17 @@ diskfs_rename_dir (struct node *fdp, str
   if (err && err != ENOENT)
     goto out;
 
+  ds = buf;
+  mutex_unlock (&fnp->lock);
+  remds = alloca (diskfs_dirstat_size);
+  err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, remds, fromcred);
+
+  if (err)
+    goto out;
+
+  assert (tmpnp == fnp);
+  diskfs_nrele (tmpnp);
+
   /* 2: Set our .. to point to the new parent */
   if (fdp != tdp)
     {
@@ -196,15 +208,9 @@ diskfs_rename_dir (struct node *fdp, str
     goto out;
 
   /* 4: Remove the entry in fdp. */
-  ds = buf;
-  mutex_unlock (&fnp->lock);
-  err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, ds, fromcred);
-  assert (tmpnp == fnp);
-  diskfs_nrele (tmpnp);
-  if (err)
-    goto out;
 
-  diskfs_dirremove (fdp, fnp, fromname, ds);
+  diskfs_dirremove (fdp, fnp, fromname, remds);
+  remds = 0;
   ds = 0;
   fnp->dn_stat.st_nlink--;
   fnp->dn_set_ctime = 1;
@@ -225,5 +231,7 @@ diskfs_rename_dir (struct node *fdp, str
     mutex_unlock (&fnp->lock);
   if (ds)
     diskfs_drop_dirstat (tdp, ds);
+  if (remds)
+    diskfs_drop_dirstat (fdp, remds);
   return err;
 }




Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Ognyan Kulev <ogi@fmi.uni-sofia.bg>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #47 received at 190732@bugs.debian.org (full text, mbox):

From: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
To: Marco Gerards <metgerards@student.han.nl>
Cc: 190732@bugs.debian.org, Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>
Subject: Re: Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Thu, 17 Jul 2003 15:04:35 +0300
On Wed, Jul 16, 2003 at 08:17:28PM +0200, Marco Gerards wrote:
> Marco Gerards <metgerards@student.han.nl> writes:
> 
> > I have a look at this patch tommorrow to check if it has the same
> > problem with my patch and send in a new patch or I'll say what the
> > problem is.
> 
> The patch worked perfectly for me!

Please run exactly the following commands and tell us what's happening 
after _the last one_.

$ mkdir d
$ cd d
$ mkdir x
$ mv x y
$ mv y x

Regards
-- 
Ognyan Kulev <ogi@{fmi.uni-sofia.bg,fsa-bg.org}>
7D9F 66E6 68B7 A62B 0FCF  EB04 80BF 3A8C A252 9782



Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Ognyan Kulev <ogi@fmi.uni-sofia.bg>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #52 received at 190732@bugs.debian.org (full text, mbox):

From: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
To: Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>
Cc: 190732@bugs.debian.org
Subject: Re: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Thu, 17 Jul 2003 15:34:18 +0300
On Wed, Jul 16, 2003 at 05:59:20AM -0500, Marcus Brinkmann wrote:
> On Wed, Jul 16, 2003 at 01:26:20AM +0300, Ognyan Kulev wrote:
> > On Tue, Jul 15, 2003 at 05:43:24AM -0500, Marcus Brinkmann wrote:
> > > 1. What errors do you still expect from the second time diskfs_lookup is
> > > invoked with REMOVE?  Is there any error that is allowed at that time?
> > > If there is, the change is correct.
> > 
> > Before the second diskfs_lookup, mutex_unlock is called.  This leaves a 
> > little time when someone could remove the node.
> 
> The node is unlocked, but the directory is not.  You can not unlink a
> file while the directory is locked.  There might be other errors to be
> concerned about?

OK, I think I find one case :-)  Between mutex_unlock and 
diskfs_lookup, someone can change the owner of the node via 
file_chown.  If the directory that contains the node is with sticky bit 
set, the call to diskfs_checkdirmod inside diskfs_lookup will fail.

> > > 2. Is it possible, and according to diskfs.h it should be, to just
> > > call dir_lookup with REMOVE only once, at the point you introduced it
> > > in your patch, and store the dirstat until it is needed for the actual
> > > disks_dirremove call.  This saves one lookup call.  Can you try such a
> > > change and look into the code if it is feasible?
> > 
> > While trying to do that now, I recalled why this cannot be done.  If
> > diskfs_lookup is called only once in the beginning, the assertion in 
> > ext2fs/dir.c:716 is triggered with the following commands:
> > 
> > $ mkdir x
> > $ mv x y
> > $ mv y x
> > 
> > It seems that dirstat of ext2fs can break when there are other 
> > activities in the directory.  It's possible that ext2fs is wrong, but I 
> > didn't look further.
> 
> Yeah, this seems to be in particular the case if the source and target
> directory are the same?  I don't have time to really crunch all that
> code until I have figured out what is safe and what is supposed to be
> safe.  The easy fix for now, and likely the thing that is really
> required, is dropping the dirstat as you did in your patch.

Imagine what happens in directory layout when the second mv is executed.  
We have one empty entry in the directory (this was x before it becomes 
y), followed by y.  So preventry (ext2fs.c/dir.c:82) points to the empty 
slot.  But before the file is actually removed, we do diskfs_direnter 
and so preventry is no longer valid, and the assertion failed.

Reading diskfs.h:313, there can be no diskfs_lookup between 
diskfs_lookup and diskfs_{drop_dirstat,dir{enter,rewrite,remove}} in the 
same directory, so the ext2fs server is OK.

I wonder if we could replace diskfs_direnter with diskfs_dirrewrite for 
the case when the directory is the same.  Probably I'll post another 
patch soon.

Regards
-- 
Ognyan Kulev <ogi@{fmi.uni-sofia.bg,fsa-bg.org}>
7D9F 66E6 68B7 A62B 0FCF  EB04 80BF 3A8C A252 9782



Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Marco Gerards <metgerards@student.han.nl>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #57 received at 190732@bugs.debian.org (full text, mbox):

From: Marco Gerards <metgerards@student.han.nl>
To: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
Cc: 190732@bugs.debian.org, Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>
Subject: Re: Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: 19 Jul 2003 00:58:40 +0200
Ognyan Kulev <ogi@fmi.uni-sofia.bg> writes:

> Please run exactly the following commands and tell us what's happening 
> after _the last one_.
> 
> $ mkdir d
> $ cd d
> $ mkdir x
> $ mv x y
> $ mv y x

I've tested this both with and without your patch. Nothing (weird)
happened.

--
Marco




Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Ognyan Kulev <ogi@fmi.uni-sofia.bg>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #62 received at 190732@bugs.debian.org (full text, mbox):

From: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
To: Marco Gerards <metgerards@student.han.nl>
Cc: 190732@bugs.debian.org, Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>
Subject: Re: Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Mon, 21 Jul 2003 17:25:04 +0300
Marco Gerards wrote:
> Ognyan Kulev <ogi@fmi.uni-sofia.bg> writes:
>>$ mkdir d
>>$ cd d
>>$ mkdir x
>>$ mv x y
>>$ mv y x
> 
> I've tested this both with and without your patch. Nothing (weird)
> happened.

(I suppose you are talking about your patch, not about mine.)  I tried 
your patch and, suprisingly for me, the assertion failure in 
ext2fs/dir.c:716 didn't trigger.  Anyway, by protocol[1] (diskfs.h:313), 
two diskfs_lookup in the same directory must have diskfs_drop_dirstat or 
diskfs_dir* between them.

[1] http://mail.gnu.org/archive/html/bug-hurd/2003-07/msg00092.html

BTW There is a possible deadlock in this function when source and 
destination parent directories are different.  Let's name them A and B. 
 Moving A/x to B/x first locks A.  If in this moment another thread 
moves B/y to A/y, then it locks B and tries to lock A and then sleeps 
waiting A to be unlocked.  The first thread continues by trying to lock 
B, but it's already locked.  I'll try to address this and other problems 
(like reverting back st_nlink when error occurs) in a "final" patch soon.

Regards
-- 
Ognyan Kulev <ogi@{fmi.uni-sofia.bg,fsa-bg.org}>
7D9F 66E6 68B7 A62B 0FCF  EB04 80BF 3A8C A252 9782




Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Ognyan Kulev <ogi@fmi.uni-sofia.bg>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #67 received at 190732@bugs.debian.org (full text, mbox):

From: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
To: Ognyan Kulev <ogi@fmi.uni-sofia.bg>, 190732@bugs.debian.org
Cc: Marco Gerards <metgerards@student.han.nl>, Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>
Subject: Re: Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Mon, 21 Jul 2003 20:31:14 +0300
Ognyan Kulev wrote:

> BTW There is a possible deadlock in this function when source and 
> destination parent directories are different.

This is wrong.  Actually, all renames are serialized 
(libdiskfs/dir-rename.c:24).

Regards
-- 
Ognyan Kulev <ogi@{fmi.uni-sofia.bg,fsa-bg.org}>
7D9F 66E6 68B7 A62B 0FCF  EB04 80BF 3A8C A252 9782




Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Marco Gerards <metgerards@student.han.nl>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #72 received at 190732@bugs.debian.org (full text, mbox):

From: Marco Gerards <metgerards@student.han.nl>
To: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
Cc: 190732@bugs.debian.org, Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>
Subject: Re: Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: 21 Jul 2003 19:45:48 +0200
Ognyan Kulev <ogi@fmi.uni-sofia.bg> writes:

> Marco Gerards wrote:
> > Ognyan Kulev <ogi@fmi.uni-sofia.bg> writes:
> >>$ mkdir d
> >>$ cd d
> >>$ mkdir x
> >>$ mv x y
> >>$ mv y x
> > I've tested this both with and without your patch. Nothing (weird)
> > happened.
> 
> (I suppose you are talking about your patch, not about mine.)  I tried
> your patch and, suprisingly for me, the assertion failure in
> ext2fs/dir.c:716 didn't trigger.  Anyway, by protocol[1]
> (diskfs.h:313), two diskfs_lookup in the same directory must have
> diskfs_drop_dirstat or diskfs_dir* between them.

I'm quite sure I've used your patch.
 
> [1] http://mail.gnu.org/archive/html/bug-hurd/2003-07/msg00092.html
> 
> BTW There is a possible deadlock in this function when source and
> destination parent directories are different.  Let's name them A and
> B. Moving A/x to B/x first locks A.  If in this moment another thread
> moves B/y to A/y, then it locks B and tries to lock A and then sleeps
> waiting A to be unlocked.  The first thread continues by trying to
> lock B, but it's already locked.  I'll try to address this and other
> problems (like reverting back st_nlink when error occurs) in a "final"
> patch soon.

I'm looking forward to your patch! :)

Thanks,
Marco




Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Ognyan Kulev <ogi@fmi.uni-sofia.bg>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #77 received at 190732@bugs.debian.org (full text, mbox):

From: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
To: 190732@bugs.debian.org
Subject: Bugs in libdiskfs/dir-rename.c
Date: Wed, 23 Jul 2003 17:45:40 +0300
While trying to solve #190732 (libdiskfs/dir-renamed.c), I found some
suspicous code in dir-rename.c.  I'm not sure if all are bugs, so I post
them here to be discussed, if needed.

1. All directory renames are serialized by renamedirlock.  It looks like
this (in a kind of pseudo-code):

again:
 lookup (fromname);
 if (fromname is directory) {
   if (! try_lock (renamedirlock)) {
     lock (renamedirlock);
     goto again;
   }
   ...
 }

As you see, if renamedirlock is held by somebody else, then no
serialization will occur.

2. When directory is renamed, dir-renamed.c::diskfs_rename_dir is
called.  After that, there are some diskfs_file_update calls when
disk_synchronous.  But these functions seem to be called in
diskfs_rename_dir too, so calling them should be redundant.

Another issue is if they have to be called when renaming regular file,
not directory.  This is not done by dir-rename.c::diskfs_S_dir_rename.

As far as I understand, diskfs_node_update is called when stat fields
are changed, and diskfs_file_update is called when (usually both) stat
fields and file content are changed.  Then why there are some
diskfs_file_update in dir-renamed.c on FNP and TNP?  (The question is
not about FDP and TDP.)  Their content is not changed, only stat fields
are changed.

3. When checking if target exists and EXCL is set ("!err && excl"), the
function doesn't return and some locks are not released.  These lines
should be appended too:

diskfs_drop_dirstat (tdp, ds);
diskfs_nrele (fnp);
mutex_unlock (&tdp->lock);
return err;

4. The "diskfs_node_update (fnp, 1)" is not guarded by "if
(diskfs_synchronous)".

5. In the "tnp && S_ISDIR (tnp)" case, one more lock should be released:

diskfs_nput (tnp);

6. In the "fnp.st_nlink == max-1" case, one more lock should be released:

if (tnp)
  diskfs_nput (tnp);


Regards
-- 
Ognyan Kulev <ogi@{fmi.uni-sofia.bg,fsa-bg.org}>
7D9F 66E6 68B7 A62B 0FCF  EB04 80BF 3A8C A252 9782





Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Ognyan Kulev <ogi@fmi.uni-sofia.bg>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #82 received at 190732@bugs.debian.org (full text, mbox):

From: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
To: 190732@bugs.debian.org
Cc: Marco Gerards <metgerards@student.han.nl>, Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>
Subject: Re: Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Sun, 27 Jul 2003 19:29:32 +0300
[Message part 1 (text/plain, inline)]
Ognyan Kulev wrote:
> I'll try to address this and other problems 
> (like reverting back st_nlink when error occurs) in a "final" patch soon.

The patch is ready, but it doesn't revert back st_nlink:s.  There are 
many small changes so it would be good if someone else take a careful 
look too.  It features verbose changelog entries too.

Regards
-- 
Ognyan Kulev <ogi@{fmi.uni-sofia.bg,fsa-bg.org}>
7D9F 66E6 68B7 A62B 0FCF  EB04 80BF 3A8C A252 9782
[dir-renamed.4.changelog (text/plain, inline)]
2003-07-27  Ognyan Kulev  <ogi@fmi.uni-sofia.bg>

	* dir-renamed.c (checkpath): Redundant assignment is removed.
	(diskfs_rename_dir): Remove variable BUF.
	Space for DS is allocated with alloca.
	When renaming node to itself, goto out instead of repeating code.
	When checking if TNP is empty directory, assertion "!err" is
	checked.
	Check early if we can remove FROMNAME.  This implicitly locks FNP,
	so the explicit locking of FNP is removed.
	When maximum link count of TDP is reached, goto out instead of
	returning immediately.
	When maximum link count of FNP is reached, goto out instead of
	repeating code.
	diskfs_node_update FNP only if diskfs_synchronous.
	After entering entry TONAME to TDP, set DS to 0.
	When preparing to remove FROMNAME from FDP, use TMPDS instead of
	DS.  Fix the assertion so that it can handle errors.  If error
	occur, diskfs_drop_dirstat TMPDS before goto out.

	* dir-rename.c (diskfs_S_dir_rename): Now really serialize
	directory renaming with RENAMEDIRLOCK.
	When renaming directory, don't diskfs_{node,file}_update because
	dir-renamed.c::diskfs_rename_dir already do it.
	When TONAME is .. of filesystem's root, not only set ERR to
	EINVAL, but return this error.
	When EXCL is set and target exist, unlock things properly.
	When FROMNAME is regular file and TONAME is directory, unlock TNP
	before returning.
	When maximum link count of FNP is reached and TNP is not NULL,
	unlock TNP before returning.
	When updating FNP after incrementing its link count, call
	diskfs_node_update only if diskfs_synchronous.
	After TONAME enters TDP, call diskfs_file_update instead of
	diskfs_node_update.
[dir-renamed.4.diff (text/plain, inline)]
diff -urpN --exclude=build --exclude='*~' --exclude=ChangeLog /home/ogi/cvs/hurd/libdiskfs/dir-rename.c hurd/libdiskfs/dir-rename.c
--- /home/ogi/cvs/hurd/libdiskfs/dir-rename.c	1997-02-14 03:19:23.000000000 +0200
+++ hurd/libdiskfs/dir-rename.c	2003-07-27 22:05:11.000000000 +0300
@@ -1,5 +1,5 @@
 /* libdiskfs implementation of fs.defs: dir_rename
-   Copyright (C) 1992, 1993, 1994, 1995, 1996, 1997 Free Software Foundation
+   Copyright (C) 1992,1993,1994,1995,1996,1997,2003 Free Software Foundation
 
    This program is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License as
@@ -52,7 +52,6 @@ diskfs_S_dir_rename (struct protid *from
   fdp = fromcred->po->np;
   tdp = tocred->po->np;
 
- try_again:
   /* Acquire the source; hold a reference to it.  This 
      will prevent anyone from deleting it before we create
      the new link. */
@@ -66,30 +65,26 @@ diskfs_S_dir_rename (struct protid *from
 
   if (S_ISDIR (fnp->dn_stat.st_mode))
     {
-      mutex_unlock (&fnp->lock);
-      if (!mutex_try_lock (&renamedirlock))
+      diskfs_nput (fnp);
+
+      mutex_lock (&renamedirlock);
+
+      /* We now lookup again, but with RENAMEDIRLOCK locked.  */
+      mutex_lock (&fdp->lock);
+      err = diskfs_lookup (fdp, fromname, LOOKUP, &fnp, 0, fromcred);
+      mutex_unlock (&fdp->lock);
+      if (err == EAGAIN)
+	err = EINVAL;
+      if (err)
 	{
-	  diskfs_nrele (fnp);
-	  mutex_lock (&renamedirlock);
-	  goto try_again;
+	  mutex_unlock (&renamedirlock);
+	  return err;
 	}
+      mutex_unlock (&fnp->lock);
+
       err = diskfs_rename_dir (fdp, fnp, fromname, tdp, toname, fromcred,
 			       tocred);
-      if (diskfs_synchronous)
-	{
-	  mutex_lock (&fdp->lock);
-	  diskfs_file_update (fdp, 1);
-	  mutex_unlock (&fdp->lock);
-	  
-	  mutex_lock (&fnp->lock);
-	  diskfs_file_update (fnp, 1);
-	  mutex_unlock (&fnp->lock);
-
-	  mutex_lock (&tdp->lock);
-	  diskfs_file_update (tdp, 1);
-	  mutex_unlock (&tdp->lock);
-	}
-      
+
       diskfs_nrele (fnp);
       mutex_unlock (&renamedirlock);
       if (!err)
@@ -107,11 +102,14 @@ diskfs_S_dir_rename (struct protid *from
   
   err = diskfs_lookup (tdp, toname, RENAME, &tnp, ds, tocred);
   if (err == EAGAIN)
-    err = EINVAL;
-  else if (!err && excl)
+    err = EINVAL;		/* .. of filesystem's root.  */
+  if (!err && excl)
     {
-      err = EEXIST;
+      diskfs_drop_dirstat (tdp, ds);
+      diskfs_nrele (fnp);
       diskfs_nput (tnp);
+      mutex_unlock (&tdp->lock);
+      return EEXIST;
     }
   if (err && err != ENOENT)
     {
@@ -138,6 +136,7 @@ diskfs_S_dir_rename (struct protid *from
     {
       diskfs_drop_dirstat (tdp, ds);
       diskfs_nrele (fnp);
+      diskfs_nput (tnp);
       mutex_unlock (&tdp->lock);
       return EISDIR;
     }
@@ -149,12 +148,15 @@ diskfs_S_dir_rename (struct protid *from
     {
       diskfs_drop_dirstat (tdp, ds);
       diskfs_nput (fnp);
+      if (tnp)
+	diskfs_nput (tnp);
       mutex_unlock (&tdp->lock);
       return EMLINK;
     }
   fnp->dn_stat.st_nlink++;
   fnp->dn_set_ctime = 1;
-  diskfs_node_update (fnp, 1);
+  if (diskfs_synchronous)
+    diskfs_node_update (fnp, 1);
 
   if (tnp)
     {
@@ -172,7 +174,7 @@ diskfs_S_dir_rename (struct protid *from
     err = diskfs_direnter (tdp, toname, fnp, ds, tocred);
 
   if (diskfs_synchronous)
-    diskfs_node_update (tdp, 1);
+    diskfs_file_update (tdp, 1);
 
   mutex_unlock (&tdp->lock);
   mutex_unlock (&fnp->lock);
@@ -185,7 +187,7 @@ diskfs_S_dir_rename (struct protid *from
   /* We now hold no locks */
 
   /* Now we remove the source.  Unfortunately, we haven't held 
-     fdp locked (nor could we), so someone else might have already
+     FDP locked (nor could we), so someone else might have already
      removed it. */
   mutex_lock (&fdp->lock);
   err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, ds, fromcred);
diff -urpN --exclude=build --exclude='*~' --exclude=ChangeLog /home/ogi/cvs/hurd/libdiskfs/dir-renamed.c hurd/libdiskfs/dir-renamed.c
--- /home/ogi/cvs/hurd/libdiskfs/dir-renamed.c	2001-10-12 05:49:17.000000000 +0300
+++ hurd/libdiskfs/dir-renamed.c	2003-07-27 20:55:45.000000000 +0300
@@ -1,5 +1,5 @@
 /*
-   Copyright (C) 1994,95,96,97,98,99,2001 Free Software Foundation, Inc.
+   Copyright (C) 1994,95,96,97,98,99,2001,03 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License as
@@ -32,7 +32,6 @@ checkpath(struct node *source,
   error_t err;
   struct node *np;
 
-  np = target;
   for (np = target, err = 0;
        /* nothing */;
        /* This special lookup does a diskfs_nput on its first argument
@@ -73,9 +72,7 @@ diskfs_rename_dir (struct node *fdp, str
 {
   error_t err;
   struct node *tnp, *tmpnp;
-  void *buf = alloca (diskfs_dirstat_size);
-  struct dirstat *ds;
-  struct dirstat *tmpds;
+  struct dirstat *ds, *tmpds;
 
   mutex_lock (&tdp->lock);
   diskfs_nref (tdp);		/* reference and lock will get consumed by
@@ -85,48 +82,57 @@ diskfs_rename_dir (struct node *fdp, str
   if (err)
     return err;
 
-  /* Now, lock the parent directories.  This is legal because tdp is not
-     a child of fnp (guaranteed by checkpath above). */
+  /* Now, lock the parent directories.  This is legal because TDP is not
+     a child of FNP (guaranteed by checkpath above). */
   mutex_lock (&fdp->lock);
   if (fdp != tdp)
     mutex_lock (&tdp->lock);
 
   /* 1: Lookup target; if it exists, make sure it's an empty directory. */
-  ds = buf;
+  ds = alloca (diskfs_dirstat_size);
   err = diskfs_lookup (tdp, toname, RENAME, &tnp, ds, tocred);
   assert (err != EAGAIN);	/* <-> assert (TONAME != "..") */
-
   if (tnp == fnp)
+    /* Renaming node to itself.  */
     {
-      diskfs_drop_dirstat (tdp, ds);
-      diskfs_nput (tnp);
-      mutex_unlock (&tdp->lock);
-      if (fdp != tdp)
-	mutex_unlock (&fdp->lock);
-      return 0;
+      err = 0;
+      fnp = 0;			/* Don't unlock the unlocked FNP.  */
+      goto out;
+    }
+  if (err && err != ENOENT)
+    {
+      fnp = 0;			/* Don't unlock the unlocked FNP.  */
+      goto out;
     }
-
-  /* Now we can safely lock fnp */
-  mutex_lock (&fnp->lock);
 
   if (tnp)
     {
+      assert (!err);
       if (! S_ISDIR(tnp->dn_stat.st_mode))
 	err = ENOTDIR;
       else if (!diskfs_dirempty (tnp, tocred))
 	err = ENOTEMPTY;
+      if (err)
+	goto out;
     }
 
-  if (err && err != ENOENT)
+  /* 2: Check permissions of the node being moved, and lock FNP.  */
+  tmpds = alloca (diskfs_dirstat_size);
+  err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, tmpds, fromcred);
+  assert (!tmpnp || tmpnp == fnp);
+  diskfs_drop_dirstat (fdp, tmpds);
+  if (tmpnp)
+    diskfs_nput (tmpnp);
+  if (err)
     goto out;
 
-  /* 2: Set our .. to point to the new parent */
+  /* 3: Set our .. to point to the new parent */
   if (fdp != tdp)
     {
       if (tdp->dn_stat.st_nlink == diskfs_link_max - 1)
 	{
 	  err = EMLINK;
-	  return EMLINK;
+	  goto out;
 	}
       tdp->dn_stat.st_nlink++;
       tdp->dn_set_ctime = 1;
@@ -157,20 +163,17 @@ diskfs_rename_dir (struct node *fdp, str
     }
 
 
-  /* 3: Increment the link count on the node being moved and rewrite
-     tdp. */
+  /* 4: Increment the link count on the node being moved and rewrite
+     TDP. */
   if (fnp->dn_stat.st_nlink == diskfs_link_max - 1)
     {
-      mutex_unlock (&fnp->lock);
-      diskfs_drop_dirstat (tdp, ds);
-      mutex_unlock (&tdp->lock);
-      if (tnp)
-	diskfs_nput (tnp);
-      return EMLINK;
+      err = EMLINK;
+      goto out;
     }
   fnp->dn_stat.st_nlink++;
   fnp->dn_set_ctime = 1;
-  diskfs_node_update (fnp, 1);
+  if (diskfs_synchronous)
+    diskfs_node_update (fnp, 1);
 
   if (tnp)
     {
@@ -188,6 +191,7 @@ diskfs_rename_dir (struct node *fdp, str
   else
     {
       err = diskfs_direnter (tdp, toname, fnp, ds, tocred);
+      ds = 0;
       if (diskfs_synchronous)
 	diskfs_file_update (tdp, 1);
     }
@@ -195,17 +199,19 @@ diskfs_rename_dir (struct node *fdp, str
   if (err)
     goto out;
 
-  /* 4: Remove the entry in fdp. */
-  ds = buf;
+  /* 5: Remove the entry from FDP. */
   mutex_unlock (&fnp->lock);
-  err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, ds, fromcred);
-  assert (tmpnp == fnp);
-  diskfs_nrele (tmpnp);
+  err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, tmpds, fromcred);
+  assert (!tmpnp || tmpnp == fnp);
+  if (tmpnp)
+    diskfs_nput (tmpnp);
   if (err)
-    goto out;
+    {
+      diskfs_drop_dirstat (fdp, tmpds);
+      goto out;
+    }
 
-  diskfs_dirremove (fdp, fnp, fromname, ds);
-  ds = 0;
+  diskfs_dirremove (fdp, fnp, fromname, tmpds);
   fnp->dn_stat.st_nlink--;
   fnp->dn_set_ctime = 1;
   if (diskfs_synchronous)

Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Ognyan Kulev <ogi@fmi.uni-sofia.bg>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #87 received at 190732@bugs.debian.org (full text, mbox):

From: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
To: 190732@bugs.debian.org
Subject: Re: Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Mon, 28 Jul 2003 16:46:32 +0300
[Message part 1 (text/plain, inline)]
Alfred M. Szmidt wrote:
> Please don't use {} in ChangeLogs.

You're right.  An updated changelog is attached.

Regards
-- 
Ognyan Kulev <ogi@{fmi.uni-sofia.bg,fsa-bg.org}>
7D9F 66E6 68B7 A62B 0FCF  EB04 80BF 3A8C A252 9782

[dir-renamed.4.changelog (text/plain, inline)]
2003-07-27  Ognyan Kulev  <ogi@magid.fmi.uni-sofia.bg>

	* dir-renamed.c (checkpath): Redundant assignment is removed.
	(diskfs_rename_dir): Remove variable BUF.
	Space for DS is allocated with alloca.
	When renaming node to itself, goto out instead of repeating code.
	When checking if TNP is empty directory, assertion "!err" is
	checked.
	Check early if we can remove FROMNAME.  This implicitly locks FNP,
	so the explicit locking of FNP is removed.
	When maximum link count of TDP is reached, goto out instead of
	returning immediately.
	When maximum link count of FNP is reached, goto out instead of
	repeating code.
	diskfs_node_update FNP only if diskfs_synchronous.
	After entering entry TONAME to TDP, set DS to 0.
	When preparing to remove FROMNAME from FDP, use TMPDS instead of
	DS.  Fix the assertion so that it can handle errors.  If error
	occur, diskfs_drop_dirstat TMPDS before goto out.

	* dir-rename.c (diskfs_S_dir_rename): Now really serialize
	directory renaming with RENAMEDIRLOCK.
	When renaming directory, don't diskfs_node_update and
	diskfs_file_update because dir-renamed.c::diskfs_rename_dir
	already do it.
	When TONAME is .. of filesystem's root, not only set ERR to
	EINVAL, but return this error.
	When EXCL is set and target exist, unlock things properly.
	When FROMNAME is regular file and TONAME is directory, unlock TNP
	before returning.
	When maximum link count of FNP is reached and TNP is not NULL,
	unlock TNP before returning.
	When updating FNP after incrementing its link count, call
	diskfs_node_update only if diskfs_synchronous.
	After TONAME enters TDP, call diskfs_file_update instead of
	diskfs_node_update.



Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Ognyan Kulev <ogi@fmi.uni-sofia.bg>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #92 received at 190732@bugs.debian.org (full text, mbox):

From: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
To: Ognyan Kulev <ogi@fmi.uni-sofia.bg>, 190732@bugs.debian.org
Cc: Marco Gerards <metgerards@student.han.nl>, Marcus Brinkmann <marcus.brinkmann@ruhr-uni-bochum.de>
Subject: Re: Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Tue, 29 Jul 2003 16:46:38 +0300
[Message part 1 (text/plain, inline)]
Ognyan Kulev wrote:
> The patch is ready, but it doesn't revert back st_nlink:s.  There are 
> many small changes so it would be good if someone else take a careful 
> look too.  It features verbose changelog entries too.

The simple patch is committed.  The long patch is updated and attached 
to this mail.  Two bugs since the last long patch are fixed (using nput 
instead of nrele and goto out with unlocked FNP).

Regards
-- 
Ognyan Kulev <ogi@{fmi.uni-sofia.bg,fsa-bg.org}>
7D9F 66E6 68B7 A62B 0FCF  EB04 80BF 3A8C A252 9782
[dir-renamed.5.changelog (text/plain, inline)]
2003-07-29  Ognyan Kulev  <ogi@fmi.uni-sofia.bg>

	* dir-renamed.c (checkpath): Redundant assignment is removed.
	(diskfs_rename_dir): Remove variable BUF.
	Space for DS is allocated with alloca.
	When renaming node to itself, goto out instead of repeating code.
	When checking if TNP is empty directory, assertion "!err" is
	checked.
	When maximum link count of TDP is reached, goto out instead of
	returning immediately.
	When maximum link count of FNP is reached, goto out instead of
	repeating code.
	diskfs_node_update FNP only if diskfs_synchronous.
	After entering entry TONAME to TDP, set DS to 0.
	When preparing to remove FROMNAME from FDP, use TMPDS instead of
	DS.  Fix the assertion so that it can handle errors.  If error
	occur, diskfs_drop_dirstat TMPDS before goto out.

	* dir-rename.c (diskfs_S_dir_rename): Now really serialize
	directory renaming with RENAMEDIRLOCK.
	When renaming directory, don't diskfs_node_update and
	diskfs_file_update because dir-renamed.c::diskfs_rename_dir
	already do it.
	When TONAME is .. of filesystem's root, not only set ERR to
	EINVAL, but return this error.
	When EXCL is set and target exist, unlock things properly.
	When FROMNAME is regular file and TONAME is directory, unlock TNP
	before returning.
	When maximum link count of FNP is reached and TNP is not NULL,
	unlock TNP before returning.
	When updating FNP after incrementing its link count, call
	diskfs_node_update only if diskfs_synchronous.
	After TONAME enters TDP, call diskfs_file_update instead of
	diskfs_node_update.
[dir-renamed.5.diff (text/plain, inline)]
diff -urpN --exclude=build --exclude='*~' --exclude=ChangeLog --exclude=CVS /home/ogi/cvs/hurd/libdiskfs/dir-rename.c hurd/libdiskfs/dir-rename.c
--- /home/ogi/cvs/hurd/libdiskfs/dir-rename.c	1997-02-14 03:19:23.000000000 +0200
+++ hurd/libdiskfs/dir-rename.c	2003-07-27 22:05:11.000000000 +0300
@@ -1,5 +1,5 @@
 /* libdiskfs implementation of fs.defs: dir_rename
-   Copyright (C) 1992, 1993, 1994, 1995, 1996, 1997 Free Software Foundation
+   Copyright (C) 1992,1993,1994,1995,1996,1997,2003 Free Software Foundation
 
    This program is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License as
@@ -52,7 +52,6 @@ diskfs_S_dir_rename (struct protid *from
   fdp = fromcred->po->np;
   tdp = tocred->po->np;
 
- try_again:
   /* Acquire the source; hold a reference to it.  This 
      will prevent anyone from deleting it before we create
      the new link. */
@@ -66,30 +65,26 @@ diskfs_S_dir_rename (struct protid *from
 
   if (S_ISDIR (fnp->dn_stat.st_mode))
     {
-      mutex_unlock (&fnp->lock);
-      if (!mutex_try_lock (&renamedirlock))
+      diskfs_nput (fnp);
+
+      mutex_lock (&renamedirlock);
+
+      /* We now lookup again, but with RENAMEDIRLOCK locked.  */
+      mutex_lock (&fdp->lock);
+      err = diskfs_lookup (fdp, fromname, LOOKUP, &fnp, 0, fromcred);
+      mutex_unlock (&fdp->lock);
+      if (err == EAGAIN)
+	err = EINVAL;
+      if (err)
 	{
-	  diskfs_nrele (fnp);
-	  mutex_lock (&renamedirlock);
-	  goto try_again;
+	  mutex_unlock (&renamedirlock);
+	  return err;
 	}
+      mutex_unlock (&fnp->lock);
+
       err = diskfs_rename_dir (fdp, fnp, fromname, tdp, toname, fromcred,
 			       tocred);
-      if (diskfs_synchronous)
-	{
-	  mutex_lock (&fdp->lock);
-	  diskfs_file_update (fdp, 1);
-	  mutex_unlock (&fdp->lock);
-	  
-	  mutex_lock (&fnp->lock);
-	  diskfs_file_update (fnp, 1);
-	  mutex_unlock (&fnp->lock);
-
-	  mutex_lock (&tdp->lock);
-	  diskfs_file_update (tdp, 1);
-	  mutex_unlock (&tdp->lock);
-	}
-      
+
       diskfs_nrele (fnp);
       mutex_unlock (&renamedirlock);
       if (!err)
@@ -107,11 +102,14 @@ diskfs_S_dir_rename (struct protid *from
   
   err = diskfs_lookup (tdp, toname, RENAME, &tnp, ds, tocred);
   if (err == EAGAIN)
-    err = EINVAL;
-  else if (!err && excl)
+    err = EINVAL;		/* .. of filesystem's root.  */
+  if (!err && excl)
     {
-      err = EEXIST;
+      diskfs_drop_dirstat (tdp, ds);
+      diskfs_nrele (fnp);
       diskfs_nput (tnp);
+      mutex_unlock (&tdp->lock);
+      return EEXIST;
     }
   if (err && err != ENOENT)
     {
@@ -138,6 +136,7 @@ diskfs_S_dir_rename (struct protid *from
     {
       diskfs_drop_dirstat (tdp, ds);
       diskfs_nrele (fnp);
+      diskfs_nput (tnp);
       mutex_unlock (&tdp->lock);
       return EISDIR;
     }
@@ -149,12 +148,15 @@ diskfs_S_dir_rename (struct protid *from
     {
       diskfs_drop_dirstat (tdp, ds);
       diskfs_nput (fnp);
+      if (tnp)
+	diskfs_nput (tnp);
       mutex_unlock (&tdp->lock);
       return EMLINK;
     }
   fnp->dn_stat.st_nlink++;
   fnp->dn_set_ctime = 1;
-  diskfs_node_update (fnp, 1);
+  if (diskfs_synchronous)
+    diskfs_node_update (fnp, 1);
 
   if (tnp)
     {
@@ -172,7 +174,7 @@ diskfs_S_dir_rename (struct protid *from
     err = diskfs_direnter (tdp, toname, fnp, ds, tocred);
 
   if (diskfs_synchronous)
-    diskfs_node_update (tdp, 1);
+    diskfs_file_update (tdp, 1);
 
   mutex_unlock (&tdp->lock);
   mutex_unlock (&fnp->lock);
@@ -185,7 +187,7 @@ diskfs_S_dir_rename (struct protid *from
   /* We now hold no locks */
 
   /* Now we remove the source.  Unfortunately, we haven't held 
-     fdp locked (nor could we), so someone else might have already
+     FDP locked (nor could we), so someone else might have already
      removed it. */
   mutex_lock (&fdp->lock);
   err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, ds, fromcred);
diff -urpN --exclude=build --exclude='*~' --exclude=ChangeLog --exclude=CVS /home/ogi/cvs/hurd/libdiskfs/dir-renamed.c hurd/libdiskfs/dir-renamed.c
--- /home/ogi/cvs/hurd/libdiskfs/dir-renamed.c	2003-07-29 01:37:24.000000000 +0300
+++ hurd/libdiskfs/dir-renamed.c	2003-07-29 19:37:04.000000000 +0300
@@ -1,5 +1,5 @@
 /*
-   Copyright (C) 1994,95,96,97,98,99,2001,2003 Free Software Foundation, Inc.
+   Copyright (C) 1994,95,96,97,98,99,2001,03 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License as
@@ -32,7 +32,6 @@ checkpath(struct node *source,
   error_t err;
   struct node *np;
 
-  np = target;
   for (np = target, err = 0;
        /* nothing */;
        /* This special lookup does a diskfs_nput on its first argument
@@ -73,9 +72,7 @@ diskfs_rename_dir (struct node *fdp, str
 {
   error_t err;
   struct node *tnp, *tmpnp;
-  void *buf = alloca (diskfs_dirstat_size);
-  struct dirstat *ds;
-  struct dirstat *tmpds;
+  struct dirstat *ds, *tmpds;
 
   mutex_lock (&tdp->lock);
   diskfs_nref (tdp);		/* reference and lock will get consumed by
@@ -85,55 +82,60 @@ diskfs_rename_dir (struct node *fdp, str
   if (err)
     return err;
 
-  /* Now, lock the parent directories.  This is legal because tdp is not
-     a child of fnp (guaranteed by checkpath above). */
+  /* Now, lock the parent directories.  This is legal because TDP is not
+     a child of FNP (guaranteed by checkpath above). */
   mutex_lock (&fdp->lock);
   if (fdp != tdp)
     mutex_lock (&tdp->lock);
 
   /* 1: Lookup target; if it exists, make sure it's an empty directory. */
-  ds = buf;
+  ds = alloca (diskfs_dirstat_size);
   err = diskfs_lookup (tdp, toname, RENAME, &tnp, ds, tocred);
   assert (err != EAGAIN);	/* <-> assert (TONAME != "..") */
-
   if (tnp == fnp)
+    /* Renaming node to itself.  */
     {
-      diskfs_drop_dirstat (tdp, ds);
-      diskfs_nput (tnp);
-      mutex_unlock (&tdp->lock);
-      if (fdp != tdp)
-	mutex_unlock (&fdp->lock);
-      return 0;
+      err = 0;
+      fnp = 0;			/* Don't unlock the unlocked FNP.  */
+      goto out;
+    }
+  if (err && err != ENOENT)
+    {
+      fnp = 0;			/* Don't unlock the unlocked FNP.  */
+      goto out;
     }
-
-  /* Check permissions to remove FROMNAME and lock FNP.  */
-  tmpds = alloca (diskfs_dirstat_size);
-  err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, tmpds, fromcred);
-  assert (!tmpnp || tmpnp == fnp);
-  if (tmpnp)
-    diskfs_nrele (tmpnp);
-  diskfs_drop_dirstat (fdp, tmpds);
-  if (err)
-    goto out;
 
   if (tnp)
     {
+      assert (!err);
       if (! S_ISDIR(tnp->dn_stat.st_mode))
 	err = ENOTDIR;
       else if (!diskfs_dirempty (tnp, tocred))
 	err = ENOTEMPTY;
+      if (err)
+	{
+	  fnp = 0;		/* Don't unlock the unlocked FNP.  */
+	  goto out;
+	}
     }
 
-  if (err && err != ENOENT)
+  /* 2: Check permissions of the node being moved, and lock FNP.  */
+  tmpds = alloca (diskfs_dirstat_size);
+  err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, tmpds, fromcred);
+  assert (!tmpnp || tmpnp == fnp);
+  if (tmpnp)
+    diskfs_nrele (tmpnp);
+  diskfs_drop_dirstat (fdp, tmpds);
+  if (err)
     goto out;
 
-  /* 2: Set our .. to point to the new parent */
+  /* 3: Set our .. to point to the new parent */
   if (fdp != tdp)
     {
       if (tdp->dn_stat.st_nlink == diskfs_link_max - 1)
 	{
 	  err = EMLINK;
-	  return EMLINK;
+	  goto out;
 	}
       tdp->dn_stat.st_nlink++;
       tdp->dn_set_ctime = 1;
@@ -164,20 +166,17 @@ diskfs_rename_dir (struct node *fdp, str
     }
 
 
-  /* 3: Increment the link count on the node being moved and rewrite
-     tdp. */
+  /* 4: Increment the link count on the node being moved and rewrite
+     TDP. */
   if (fnp->dn_stat.st_nlink == diskfs_link_max - 1)
     {
-      mutex_unlock (&fnp->lock);
-      diskfs_drop_dirstat (tdp, ds);
-      mutex_unlock (&tdp->lock);
-      if (tnp)
-	diskfs_nput (tnp);
-      return EMLINK;
+      err = EMLINK;
+      goto out;
     }
   fnp->dn_stat.st_nlink++;
   fnp->dn_set_ctime = 1;
-  diskfs_node_update (fnp, 1);
+  if (diskfs_synchronous)
+    diskfs_node_update (fnp, 1);
 
   if (tnp)
     {
@@ -195,6 +194,7 @@ diskfs_rename_dir (struct node *fdp, str
   else
     {
       err = diskfs_direnter (tdp, toname, fnp, ds, tocred);
+      ds = 0;
       if (diskfs_synchronous)
 	diskfs_file_update (tdp, 1);
     }
@@ -202,18 +202,19 @@ diskfs_rename_dir (struct node *fdp, str
   if (err)
     goto out;
 
-  /* 4: Remove the entry in fdp. */
-  ds = buf;
+  /* 5: Remove the entry from FDP. */
   mutex_unlock (&fnp->lock);
-  err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, ds, fromcred);
+  err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, tmpds, fromcred);
   assert (!tmpnp || tmpnp == fnp);
   if (tmpnp)
     diskfs_nrele (tmpnp);
   if (err)
-    goto out;
+    {
+      diskfs_drop_dirstat (fdp, tmpds);
+      goto out;
+    }
 
-  diskfs_dirremove (fdp, fnp, fromname, ds);
-  ds = 0;
+  diskfs_dirremove (fdp, fnp, fromname, tmpds);
   fnp->dn_stat.st_nlink--;
   fnp->dn_set_ctime = 1;
   if (diskfs_synchronous)

Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to bddebian@cox.net:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #97 received at 190732@bugs.debian.org (full text, mbox):

From: Barry deFreese <bddebian@cox.net>
To: Marco Gerards <metgerards@student.han.nl>, 190732@bugs.debian.org
Subject: Bug List
Date: Sat, 02 Aug 2003 06:40:14 -0700
[Message part 1 (text/plain, inline)]
Marco,

Here is the bug/patch list that I put together.  Any input is welcome.

Thanks!!



-- 
Barry deFreese
Debian 3.0r1 "Woody"
Registered Linux "Newbie" #302256 - Debian Developer Wannabe

"Programming today is a race between software engineers striving
to build bigger and better idiot-proof programs, and the Universe
trying to produce bigger and better idiots. So far, the Universe is
winning." Rich Cook.




[consolidatedbugslistv3.txt (text/plain, inline)]
-------------------------------------------------------------------------
Title:  [BUG] Glibc without installed headers  
Date: July 2002
Submitter:  Jeff Bailey
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-07/msg00265.html
Status: Open
Description:
  make install-headers no_deps=t, required for the full bootstrap, does
  fail.
  Previously all headers the Hurd installed were simple files to be copied.
  Now trivfs has some headers that are mig-generated, and you can't do mig
  generation without a proper full set of headers installed.
Action:
  I think what we should do is have install-headers, or a differently-named
  new target, install just the plain headers and not the generated ones.
  Or it might be sufficient just to do make install-headers in the hurd
  subdir to build libc.  Try that.


-------------------------------------------------------------------------
Title:  [BUG] Bug#151407: hurd: mount can't parse dselect command  
Date: July 2002
Submitter:  Marcus Brinkmann
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-07/msg00001.html
Status: Unable to reproduce [Barry]
Description:
  mount doesn't support mixing argument options with non-argument options
  in the same dash, like this:  mount -rt fs
  which is equivalent to:       mount -r -t fs
Action:
  Fix option parsing in mount to allow several short options with one dash.

Barry deFreese:
I cannot reproduce this problem and -r isn't a valid parameter to mount??
Using mount -R -t iso9660fs gives no results.
Using mount -Rt iso9660fs also gives no results.


-------------------------------------------------------------------------
Title:  [BUG] "renice" weird behaviour  
Date:  August 2002
Submitter:  Ludovic Courths
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-08/msg00013.html
Status: Open [Barry]
Description:
  nice() is broken.
Action:
  Fix needs to diddle Mach's priorities for the task and the threads.
  This includes the priorities for the processor set the threads run in.


-------------------------------------------------------------------------
Title:  [BUG] storeio crashes with gunzip/bunzip2 classes  
Submitter:  Ludovic Courths
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-08/msg00078.html
Status: Unable to reproduce [Barry]
Description:
  Unable to set storeio translator on gunzip and bunzip2 store classes.
Action:
  Fix it.

Barry deFreese:
I was unable to reproduce this problem.
Running:

  $ settrans -ca t /hurd/storeio -T gunzip file.gz
  $ cat t

Gives empty file for t with no errors.


-------------------------------------------------------------------------
Title:  [BUG] debuild check for fakeroot fails on GNU/Hurd  
Date:  August 2002
Submitter:  James Morrison
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-08/msg00191.html
Status: Unknown [Barry]
Description:
  Currently debuild check if fakeroot is available by running 'fakeroot :'. 
  This fails on GNU/Hurd because fakeroot uses exec $*.
Action:
  Remove call to exec().
  Potentially change GNU/Linux fakeroot to use "$@" rather than "sh -c $".


-------------------------------------------------------------------------
Title:  [BUG] showtrans --active  
Date:  August 2002
Submitter:  David Walter
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-08/msg00194.html
Status: Unknown [Barry]
Description:
  Cannot determine the active translator running on an inode.
Action:
  Determine if this is truly a bug.


-------------------------------------------------------------------------
Title:  [BUG] Bug#156600: FTBFS: hurd should build-depend on autoconf2.13 
Date:  August 2002
Submitter:  James A Morrison
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-08/msg00209.html
Status: Open [Barry]
Description:
  The autoconf package no longer provides autoconf2.13.  So it can be
  built from source.
Action:
  Test patch submitted by James and fix if appropriate.  See archive.


-------------------------------------------------------------------------
Title:  [PATCH] mtab translator 
Date:  August 2002
Submitter:  David Walter
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-08/msg00238.html
Status: Unknown [Barry]
Description:
  Code to create mtab type translator for GNU/Hurd.
Action:
  Review patch.  Test.  Fix.  See patch here:
  http://mail.gnu.org/archive/html/bug-hurd/2002-08/msg00244.html


-------------------------------------------------------------------------
Title:  [BUG] Interrupted system call 
Date:  August 2002
Submitter:  Wolfgang Jahrling
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-08/msg00257.html
Status: Unable to reproduce. [Barry]
Description:
  Intermittent interrupted system calls in configure.
Action:
  Need to attempt to reproduce problem.
  Debug.
  Fix it!


-------------------------------------------------------------------------
Title:  [PATCH] mtab: libdiskfs / ext2fs support changes 
Date:  September 2002
Submitter:  David Walter
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-09/msg00038.html
Status: Unable to reproduce. [Barry]
Description:
  libdiskfs patch for mtab patch from earlier archive.  See message here:
  http://mail.gnu.org/archive/html/bug-hurd/2002-08/msg00238.html
Action:
  Test patch.  Fix if appropriate and commit.


-------------------------------------------------------------------------
Title:  [PATCH] Fixes issue with oskit-libdir when compiling oskit-mach 
Date:  September 2002
Submitter:  Michal 'hramrach' Suchanek
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-09/msg00063.html
Status: Open [Barry]
Description:
  This patch will fix an issue with oskit-libdir when compiling GNUmach 2.x
Action:
  Test patch.
  Commit.


-------------------------------------------------------------------------
Title:  [BUG] Increase stack size in cthreads
Date:  September 2002
Submitter:  Neal H. Walfield
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-09/msg00141.html
Status: Open [Barry]
Description:
  Should stack size in cthreads be increased?
Action:
  Move to pthreads.  See here:
  http://mail.gnu.org/archive/html/bug-hurd/2002-09/msg00144.html


-------------------------------------------------------------------------
Title:  [BUG] float format, pointer arg - nfsd/fsys.c
Date:  September 2002
Submitter:  Marcus Brinkmann
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-09/msg00175.html
Status: Open [Barry]
Description:
  Recieve the following error in nfsd:
  warning: float format, pointer arg (arg 4)
Action:
  Requires new modifier for nitems = fscanf (index_file, "%d %as\n", &index, &name);
  Need to determine possibilities for replacing 'a' modifier before %s.


-------------------------------------------------------------------------
Title:  [BUG] Parted seg faults
Date:  October 2002
Submitter:  James Morrison
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-10/msg00036.html
Status: Open [Barry]
Description:
  Parted with no arguments gives segmentation fault.
Action:
  Crashes in dlsym call. Debug and test.

7/31/2003: [Barry deFreese]
I have parted v 1.6.4 and ran parted with no arguments and recived a segmenation fault.


-------------------------------------------------------------------------
Title:  [BUG] fork() loses when thread self port's refcount is max'ed out
Date:  October 2002
Submitter:  Marcus Brinkmann
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-10/msg00098.html
Status: Open [Barry]
Description:
  fork() leaks port rights.
Action:
  See archive for fix to programs that leak port rights.


-------------------------------------------------------------------------
Title:  [BUG] arbitrary IDs with available UID 0?
Date:  November 2002
Submitter:  Marcus Brinkmann
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-11/msg00112.html
Status: Open [Barry]
Description:
  The doc says that you are allowed to create auth objects associated with any
  IDs if you have euid 0, and the code actually allows it even if only auid 0.
Action:
  Thomas suggests fixing the code to not use auids.


-------------------------------------------------------------------------
Title:  [PATCH] gnumach2 and pcmcia
Date:  November 2002
Submitter:  Daniel Wagner
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-11/msg00131.html
Status: Open [Barry]
Description:
  Daniel has created a patch for pcmcia under GNUmach 2.x.
Action:
  Test and debug.


-------------------------------------------------------------------------
Title:  [BUG] argp crashes because of CTYPE_B threadvar being 0
Date:  November 2002
Submitter:  Marcus Brinkmann
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-11/msg00221.html
Status: Possibly fixed in later glibc package [Barry]
Description:
  the glibc 2.3 has another problem, which is exhibited by fsysopts on a
  server.  The problem shows up because argp uses isprint, and the new
  implementation of that uses ctype_b, which is a thread local variable.
  However, this variable is only set for the main thread, all other thread
  have this variable set to 0.  Thus isprint crashes.
Action:
  Verify that this bug does not still exist.


-------------------------------------------------------------------------
Title:  [PATCH] proxy memory objects patch
Date:  November 2002
Submitter:  Marcus Brinkmann
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-11/msg00247.html
Status: Possibly closed [Barry]
Description:
  Patch for proxy memory objects for GNUmach 1.x
Action:
  Verify that patch has been committed.


-------------------------------------------------------------------------
Title:  [BUG] Filesystem corruption bug in libdiskfs
Date:  December 2002
Submitter:  Richard Smith
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-12/msg00072.html
Status: Unknown [Barry]
Description:
  In diskfs_rename_dir, filesystem corruption can result if a directory is
  moved that by a user who cannot write to the parent directory.
Action:
  Verify patch and test.

7/31/2003: - [Barry]
Should be committed but I don't see it in the ChangeLog for libdiskfs. - Barry


-------------------------------------------------------------------------
Title:  [PATCH] don't test the OSENV_NONBLOCKING flag
Date:  December 2002
Submitter:  Marcus Brinkmann
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-12/msg00092.html
Status: Unknown [Barry]
Description:
  Partial patch to osenv_mem.c applied and needs to be fully applied.
Action:
  Verify this is still an issue.
  Commit full patch.

7/31/2003: - [Barry]
I don't see this one either but it looks as though osenv_mem.c may have been redone??


-------------------------------------------------------------------------
Title:  [BUG] empty symlinks, e2fsck
Date:  December 2002
Submitter:  Moritz Schulte
URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-12/msg00181.html
Status: Unknown [Barry]
Description:
  Empty symlinks not handled properly by e2fsck.
  (Show as inconsistencies).
Action:
  Either ignore empty symlinks in e2fsck or handle them properly.
  Maybe see how GNU/Linux handles them?


-------------------------------------------------------------------------
Title:  [PATCH] soft interrupts 
Date:  January 2003
Submitter:  Daniel Wagner
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-01/msg00088.html
Status: Unknown [Barry]
Description:
  At the moment the softint handler is called from softclock handler.
  This leads to a panic.
Action:
  Apply patch and test.

7/31/2003: - [Barry]
Patch code not in: osenv_softirq.c but this is 1.3 right??


-------------------------------------------------------------------------
Title:  [PATCH] gnumach2 & the serial port 
Date:  March 2003
Submitter:  Daniel Wagner
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-03/msg00015.html
Status: Unknown [Barry]
Description:
  Freebsd serial i/o driver for GNUmach 2.x
Action:
  Apply patch and test.


-------------------------------------------------------------------------
Title:  [BUG] libc0.3: missing shm* functions (from <sys/shm.h>) 
Date:  March 2003
Submitter:  Robert Millan
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-03/msg00041.html
Status: Unknown [Barry]
Description:
  Functions from sys/shm.h are not implemented in libc0.3.
Action:
  Verify that this bug is still valid.
  Implement functions.


-------------------------------------------------------------------------
Title:  [BUG] libc0.3-dev: weirdness with sockaddr_un 
Date:  April 2003
Submitter:  Robert Millan
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-04/msg00001.html
Status: Unknown [Barry]
Description:

Don't know if this ever got defined as a "bug" or not, looked more like just 
discussion???  I wasn't clear. - Barry


-------------------------------------------------------------------------
Title:  [BUG] libc0.3-dev: fcntl F_GETLK not implemented (ENOSYS) 
Date:  April 2003
Submitter:  Robert Millan
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-04/msg00105.html
Status: Unknown [Barry]
Description:


Worth a bug-report? - Barry


-------------------------------------------------------------------------
Title:  [PATCH] fatfs virtual inodes 
Date:  April 2003
Submitter:  Marco Gerards
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-04/msg00152.html
Status: Open [Barry]
Description:
  Patch for virtual inodes for fatfs.
Action:
  Test patch and apply.


7/31/2003: - [Barry]
Doesn't appear committed yet.


-------------------------------------------------------------------------
Title:  [BUG] Bug while calling dlcose 
Date:  May 2003
Submitter:  Marco Gerards
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-05/msg00095.html
Status: Unknown [Barry]
Description:


Not sure this was ever classified as a "bug" or not? - Barry


-------------------------------------------------------------------------
Title:  [PATCH] Patch to CVS OSKIT to Compile CVS oskit-mach 
Date:  June 2003
Submitter:  Michael Oberg
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-06/msg00054.html
Status: Unknown [Barry]
Description:
  This patch adds oskit/libc/string/stpcpy.c to the OSKIT source.
Action:
  Test and apply.


Not much info on this one??? - Barry


-------------------------------------------------------------------------
Title:  [BUG] Kernel Page Fault in CVS oskit-mach
Date:  June 2003
Submitter:  Michael Oberg
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-06/msg00057.html
Status: Open [Barry]
Description:
  After compiling GNUmach 2.x, receive kernel page faults.
  With or without inclusion of ethernet driver.
Action:
  Determine root cause.  Micheal has done some preliminary debugging.
  Fix it!

7/31/2003: - [Barry]
I have this same problem building oskit-mach with ethernet_vortex...


-------------------------------------------------------------------------
Title:  [PATCH] Translator support for GNU tar
Date:  June 2003
Submitter:  Marco Gerards 
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-06/msg00069.html
Status: Open [Barry]
Description:
  Patch for GNU tar to support passive translators.
Action:
  Test patch and commit.


-------------------------------------------------------------------------
Title:  [PATCH] dhcp client (pfinet patch)
Date:  June 2003
Submitter:  Marco Gerards 
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-06/msg00079.html
Status: Open [Barry]
Description:
  Patch to add dhcp support to pfinet translator.
Action:
  Test patch.
  Need to fix other known problems, contact Marco.

7/31/2003: - [Barry]
I tried to test this previously but it still had some bugs that Marco knew about.
Not sure of the current status?


-------------------------------------------------------------------------
Title:  [BUG] Hurd program output for --help, --usage, and --version
Date:  June 2003
Submitter:  Barry deFreese 
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-06/msg00096.html
Status: Open [Barry]
Description:
  Several Hurd programs do not handle default argp arguments.
Action:
  Add argp support to programs listed.  See archive for list of programs.


-------------------------------------------------------------------------
Title:  [PATCH] Patch to add argp options to fwd translator
Date:  June 2003
Submitter:  Barry deFreese 
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-06/msg00119.html
Status: Open [Barry]
Description:
  Added standard argp handling to the fwd translator.
Action:
  Test patch.
  Needs ChangeLog entry.

7/31/2003: - [Barry]
Someone may have to pick this up for me.  I am still having problems with legal
and don't have my release or assignment papers yet.


-------------------------------------------------------------------------
Title:  [BUG] stpcpy?
Date:  July 2003
Submitter:  Derek Davies 
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-07/msg00001.html
Status: Open [Barry]
Description:
  Unresolved refs to stpcpy when building oskit/mach from CVS.
Action:
  Patch was sent by Michael Oberg.
  Test patch and apply.

7/31/2003: - [Barry]
Should patch mentioned be committed? No ChangeLog entry found.


-------------------------------------------------------------------------
Title:  [PATCH] Fatfs patch for writing support 
Date:  July 2003
Submitter:  Marco Gerards
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-07/msg00014.html
Status: Open [Barry]
Description:
  Patch for FAT32 reading support in fatfs.
Action:
  Test patch and apply.

7/31/2003: - [Barry]
Patch doesn't appear to have been committed.


-------------------------------------------------------------------------
Title:  [PATCH] hello.c follow-up 
Date:  July 2003
Submitter:  Julien PUYDT
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-07/msg00109.html
Status: Open [Barry]
Description:
  Updates/fixes to hello.c
Action:
  Test patch and apply.

7/31/2003: - [Barry]
Not committed yet as near as I can tell.


-------------------------------------------------------------------------
Title:  [PATCH] Patch for ncursesw (bug-fixes, other resolutions)  
Date:  July 2003
Submitter:  Marco Gerards
URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-07/msg00112.html
Status: Open [Barry]
Description:
  This patch adds support for non-80x25 resolutions to the ncursesw
  driver. It also adds a generic interface to tell the display driver
  which resolution to use. If that resolution is not available it uses
  the next best resolution.
Action:
  Test patch and apply.


7/31/2003: - [Barry]
Not committed.  I believe there was some discussion between Marco and Marcus?


Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to PUYDT Julien <julien.puydt@laposte.net>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #102 received at 190732@bugs.debian.org (full text, mbox):

From: PUYDT Julien <julien.puydt@laposte.net>
To: 190732@bugs.debian.org
Subject: Re: Bug#190732: Bug List
Date: 03 Aug 2003 10:22:43 +0200
Le sam 02/08/2003 à 15:40, Barry deFreese a écrit :
> Marco,
> 
> Here is the bug/patch list that I put together.  Any input is welcome.
> 
> Thanks!!

About the hello.c patch I proposed: I will remake it for the following
reasons:
* Jeroen Dekkers gave feedback on the code, that will get used;
* Alfred M. Szmidt gave feedback on the ChangeLog, that won't get used
since:
* there really are three different sets of change in that single patch,
so I'm gonna break it in several;
* I would like to add comments with the functions I add;

this will have to wait a little, as I'm already busy on another
project... that isn't urgent, as it just is about having a nice source
for beginners (like me...).

Snark on #hurd, #hurdfr




Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Marco Gerards <metgerards@student.han.nl>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #107 received at 190732@bugs.debian.org (full text, mbox):

From: Marco Gerards <metgerards@student.han.nl>
To: bddebian@cox.net
Cc: 190732@bugs.debian.org
Subject: Re: Bug List
Date: 03 Aug 2003 23:26:14 +0200
Barry deFreese <bddebian@cox.net> writes:

> Marco,
> 
> Here is the bug/patch list that I put together.  Any input is welcome.
> 
> Thanks!!
> 
> 
> 
> -- 
> Barry deFreese
> Debian 3.0r1 "Woody"
> Registered Linux "Newbie" #302256 - Debian Developer Wannabe
> 
> "Programming today is a race between software engineers striving
> to build bigger and better idiot-proof programs, and the Universe
> trying to produce bigger and better idiots. So far, the Universe is
> winning." Rich Cook.

> -------------------------------------------------------------------------
> Title:  [PATCH] mtab translator 
> Date:  August 2002
> Submitter:  David Walter
> URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-08/msg00238.html
> Status: Unknown [Barry]
> Description:
>   Code to create mtab type translator for GNU/Hurd.
> Action:
>   Review patch.  Test.  Fix.  See patch here:
>   http://mail.gnu.org/archive/html/bug-hurd/2002-08/msg00244.html

I'd like such translator and I'm willing to have a look at it...


> -------------------------------------------------------------------------
> Title:  [PATCH] gnumach2 and pcmcia
> Date:  November 2002
> Submitter:  Daniel Wagner
> URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-11/msg00131.html
> Status: Open [Barry]
> Description:
>   Daniel has created a patch for pcmcia under GNUmach 2.x.
> Action:
>   Test and debug.

Isn't this in CVS already in the OSKIT project on savannah?
 
> -------------------------------------------------------------------------
> Title:  [BUG] Filesystem corruption bug in libdiskfs
> Date:  December 2002
> Submitter:  Richard Smith
> URL:  http://mail.gnu.org/archive/html/bug-hurd/2002-12/msg00072.html
> Status: Unknown [Barry]
> Description:
>   In diskfs_rename_dir, filesystem corruption can result if a directory is
>   moved that by a user who cannot write to the parent directory.
> Action:
>   Verify patch and test.
> 
> 7/31/2003: - [Barry]
> Should be committed but I don't see it in the ChangeLog for libdiskfs. - Barry


 2003-06-11  Ognyan Kulev  <ogi@fmi.uni-sofia.bg>

        * dir-renamed.c (diskfs_rename_dir): Check permissions to remove
        FROMNAME before any modification could take place.  Check result
        of removing the from node.

:))

> -------------------------------------------------------------------------
> Title:  [PATCH] fatfs virtual inodes 
> Date:  April 2003
> Submitter:  Marco Gerards
> URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-04/msg00152.html
> Status: Open [Barry]
> Description:
>   Patch for virtual inodes for fatfs.
> Action:
>   Test patch and apply.
> 
> 
> 7/31/2003: - [Barry]
> Doesn't appear committed yet.

It's not sure if it is useful or not. Marcus and I should talk about
this some day. Low priority.
 
> -------------------------------------------------------------------------
> Title:  [BUG] Bug while calling dlcose 
> Date:  May 2003
> Submitter:  Marco Gerards
> URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-05/msg00095.html
> Status: Unknown [Barry]
> Description:
> 
> 
> Not sure this was ever classified as a "bug" or not? - Barry

It is a bug... *somewhere*

> -------------------------------------------------------------------------
> Title:  [PATCH] Translator support for GNU tar
> Date:  June 2003
> Submitter:  Marco Gerards 
> URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-06/msg00069.html
> Status: Open [Barry]
> Description:
>   Patch for GNU tar to support passive translators.
> Action:
>   Test patch and commit.

I will send in a new patch that fixes some issues.
 
> -------------------------------------------------------------------------
> Title:  [PATCH] dhcp client (pfinet patch)
> Date:  June 2003
> Submitter:  Marco Gerards 
> URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-06/msg00079.html
> Status: Open [Barry]
> Description:
>   Patch to add dhcp support to pfinet translator.
> Action:
>   Test patch.
>   Need to fix other known problems, contact Marco.
> 
> 7/31/2003: - [Barry]
> I tried to test this previously but it still had some bugs that Marco knew about.
> Not sure of the current status?

New patches have been put on savannah. This patch is obsolete.
 
> -------------------------------------------------------------------------
> Title:  [PATCH] Fatfs patch for writing support 
> Date:  July 2003
> Submitter:  Marco Gerards
> URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-07/msg00014.html
> Status: Open [Barry]
> Description:
>   Patch for FAT32 reading support in fatfs.
> Action:
>   Test patch and apply.
> 
> 7/31/2003: - [Barry]
> Patch doesn't appear to have been committed.

Commited!
 
> -------------------------------------------------------------------------
> Title:  [PATCH] Patch for ncursesw (bug-fixes, other resolutions)  
> Date:  July 2003
> Submitter:  Marco Gerards
> URL:  http://mail.gnu.org/archive/html/bug-hurd/2003-07/msg00112.html
> Status: Open [Barry]
> Description:
>   This patch adds support for non-80x25 resolutions to the ncursesw
>   driver. It also adds a generic interface to tell the display driver
>   which resolution to use. If that resolution is not available it uses
>   the next best resolution.
> Action:
>   Test patch and apply.
> 
> 
> 7/31/2003: - [Barry]
> Not committed.  I believe there was some discussion between Marco and Marcus?

Yes, it is on savannah now.

Thanks,
Marco




Tags added: fixed Request was from Ognyan Kulev <ogi@fmi.uni-sofia.bg> to control@bugs.debian.org. Full text and rfc822 format available.

Tags removed: fixed Request was from Robert Millan <zeratul2@wanadoo.es> to control@bugs.debian.org. Full text and rfc822 format available.

Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Ognyan Kulev <ogi@fmi.uni-sofia.bg>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #116 received at 190732@bugs.debian.org (full text, mbox):

From: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
To: Marcus Brinkmann <Marcus.Brinkmann@ruhr-uni-bochum.de>, 156600@bugs.debian.org, 190732@bugs.debian.org
Cc: Robert Millan <zeratul2@wanadoo.es>
Subject: Re: Bug#156600: Processed: Partially fixed
Date: Mon, 18 Aug 2003 17:59:07 +0300
I've just noticed that all this is logged in bug #156600.  But we are 
talking about #190732.

This mail is just to mark in #190732 that this bug should be closed when 
new hurd package is uploaded.

Regards
-- 
Ognyan Kulev <ogi@{fmi.uni-sofia.bg,fsa-bg.org}>
7D9F 66E6 68B7 A62B 0FCF  EB04 80BF 3A8C A252 9782




Tags added: fixed-upstream Request was from Robert Millan <zeratul2@wanadoo.es> to control@bugs.debian.org. Full text and rfc822 format available.

Tags added: pending Request was from Robert Millan <zeratul2@wanadoo.es> to control@bugs.debian.org. Full text and rfc822 format available.

Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Nathanael Nerode <neroden@twcny.rr.com>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #125 received at 190732@bugs.debian.org (full text, mbox):

From: Nathanael Nerode <neroden@twcny.rr.com>
To: 190732@bugs.debian.org
Subject: Bug fixed, maybe?
Date: Wed, 24 Dec 2003 21:56:04 -0500
Um, it looks like new versions of 'hurd' have been uploaded several 
times since this bug was marked 'pending'.  Is it actually fixed; can it 
be closed?  (If not, why is it still marked 'pending'?)




Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Santiago Vila <sanvila@unex.es>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #130 received at 190732@bugs.debian.org (full text, mbox):

From: Santiago Vila <sanvila@unex.es>
To: Nathanael Nerode <neroden@twcny.rr.com>, 190732@bugs.debian.org
Subject: Re: Bug#190732: Bug fixed, maybe?
Date: Thu, 25 Dec 2003 14:12:25 +0100 (CET)
On Wed, 24 Dec 2003, Nathanael Nerode wrote:

> Um, it looks like new versions of 'hurd' have been uploaded several
> times since this bug was marked 'pending'.  Is it actually fixed; can it
> be closed?  (If not, why is it still marked 'pending'?)

This bug was reported in April 2003, and the last official hurd.deb package
in the archives has version 20021118-1. We are waiting for somebody (Jeff?)
to update the debian package from the CVS source.



Tags removed: pending Request was from neroden@twcny.rr.com (Nathanael Nerode) to control@bugs.debian.org. Full text and rfc822 format available.

Reply sent to Michael Banck <mbanck@debian.org>:
You have taken responsibility. Full text and rfc822 format available.

Notification sent to Robert Millan <rmh@debian.org>:
Bug acknowledged by developer. Full text and rfc822 format available.

Message #137 received at 190732-done@bugs.debian.org (full text, mbox):

From: Michael Banck <mbanck@debian.org>
To: 190732-done@bugs.debian.org
Subject: Re: hurd: non-priviledged user may crash filesystem
Date: Mon, 1 Mar 2004 19:18:16 +0100
This bug has been fixed by the recent upload of hurd_20040301-1. One
patch has been applied to fix this bug:

2003-06-11  Ognyan Kulev  <ogi@fmi.uni-sofia.bg>

        * dir-renamed.c (diskfs_rename_dir): Check permissions to remove
        FROMNAME before any modification could take place.  Check result
        of removing the from node.

The still not applied patch for libdiskfs contained in the bug log is
also available at
http://savannah.gnu.org/patch/?func=detailitem&item_id=1839


Thanks,

Michael



Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to "Alfred M. Szmidt" <ams@kemisten.nu>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #142 received at 190732@bugs.debian.org (full text, mbox):

From: "Alfred M. Szmidt" <ams@kemisten.nu>
To: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
Cc: ogi@fmi.uni-sofia.bg, 190732@bugs.debian.org, metgerards@student.han.nl, marcus.brinkmann@ruhr-uni-bochum.de
Subject: Re: Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Tue, 2 Mar 2004 17:03:44 +0100 (MET)
   2003-07-29  Ognyan Kulev  <ogi@fmi.uni-sofia.bg>

	   * dir-renamed.c (checkpath): Redundant assignment is removed.
	   (diskfs_rename_dir): Remove variable BUF.
	   Space for DS is allocated with alloca.
	   When renaming node to itself, goto out instead of repeating code.
	   When checking if TNP is empty directory, assertion "!err" is
	   checked.
	   When maximum link count of TDP is reached, goto out instead of
	   returning immediately.
	   When maximum link count of FNP is reached, goto out instead of
	   repeating code.
	   diskfs_node_update FNP only if diskfs_synchronous.
	   After entering entry TONAME to TDP, set DS to 0.
	   When preparing to remove FROMNAME from FDP, use TMPDS instead of
	   DS.  Fix the assertion so that it can handle errors.  If error
	   occur, diskfs_drop_dirstat TMPDS before goto out.

	   * dir-rename.c (diskfs_S_dir_rename): Now really serialize
	   directory renaming with RENAMEDIRLOCK.
	   When renaming directory, don't diskfs_node_update and
	   diskfs_file_update because dir-renamed.c::diskfs_rename_dir
	   already do it.
	   When TONAME is .. of filesystem's root, not only set ERR to
	   EINVAL, but return this error.
	   When EXCL is set and target exist, unlock things properly.
	   When FROMNAME is regular file and TONAME is directory, unlock TNP
	   before returning.
	   When maximum link count of FNP is reached and TNP is not NULL,
	   unlock TNP before returning.
	   When updating FNP after incrementing its link count, call
	   diskfs_node_update only if diskfs_synchronous.
	   After TONAME enters TDP, call diskfs_file_update instead of
	   diskfs_node_update.

What bug did this patch fix exactly?  Do you have any test cases for
this?

And for the future, please please please refrain from obfuscuating the
patch with cosmetic changes (like removing redundat variables and what
not).  It makes it so much harder to read and follow.



Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Ognyan Kulev <ogi@fmi.uni-sofia.bg>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #147 received at 190732@bugs.debian.org (full text, mbox):

From: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
To: "Alfred M. Szmidt" <ams@kemisten.nu>
Cc: 190732@bugs.debian.org, metgerards@student.han.nl, marcus.brinkmann@ruhr-uni-bochum.de
Subject: Re: Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Tue, 02 Mar 2004 20:18:08 +0200
Debian Bug #190732 can be safely closed.  The patch you're talking about 
is in http://sv.gnu.org/patch/?func=detailitem&item_id=1839 too.

Alfred M. Szmidt wrote:
> What bug did this patch fix exactly?  Do you have any test cases for
> this?

Many small bugs are fixed.  Most of the sentences in the changelog entry 
are separate fixes that are not related to the other changelog entries. 
 If it's needed, I can make more detailed explanation about each sentence.

> And for the future, please please please refrain from obfuscuating the
> patch with cosmetic changes (like removing redundat variables and what
> not).  It makes it so much harder to read and follow.

My observation is that such cosmetic changes (like changing comments and 
memset/memcpy) are rejected by Hurd core developers when they are alone, 
so the only way to commit them is to merge them with more essential patch.

Regards,
ogi




Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to "Alfred M. Szmidt" <ams@kemisten.nu>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #152 received at 190732@bugs.debian.org (full text, mbox):

From: "Alfred M. Szmidt" <ams@kemisten.nu>
To: Ognyan Kulev <ogi@fmi.uni-sofia.bg>
Cc: 190732@bugs.debian.org, metgerards@student.han.nl, marcus.brinkmann@ruhr-uni-bochum.de
Subject: Re: Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Tue, 2 Mar 2004 20:38:42 +0100 (MET)
   Debian Bug #190732 can be safely closed.  The patch you're talking
   about is in http://sv.gnu.org/patch/?func=detailitem&item_id=1839
   too.

Okie.

   > What bug did this patch fix exactly?  Do you have any test cases
   > for this?

   Many small bugs are fixed.  Most of the sentences in the changelog
   entry are separate fixes that are not related to the other
   changelog entries.  If it's needed, I can make more detailed
   explanation about each sentence.

If you could, please do since it would be nice to get this into the
tree if it fixes bugs.

   > And for the future, please please please refrain from
   > obfuscuating the patch with cosmetic changes (like removing
   > redundat variables and what not).  It makes it so much harder to
   > read and follow.

   My observation is that such cosmetic changes (like changing
   comments and memset/memcpy) are rejected by Hurd core developers
   when they are alone, so the only way to commit them is to merge
   them with more essential patch.

I don't think they will reject them if they are sane and actually
clean up the code.  And if that doesn't work, just point to some
change by the gods that actually did a clean up. :)

Anyway, I find it more important to get actual fixes in, and a patch
that one can read without some unneeded cruft is really helpful.



Information forwarded to debian-bugs-dist@lists.debian.org, GNU Hurd Maintainers <bug-hurd@gnu.org>:
Bug#190732; Package hurd. Full text and rfc822 format available.

Acknowledgement sent to Roland McGrath <roland@frob.com>:
Extra info received and forwarded to list. Copy sent to GNU Hurd Maintainers <bug-hurd@gnu.org>. Full text and rfc822 format available.

Message #157 received at 190732@bugs.debian.org (full text, mbox):

From: Roland McGrath <roland@frob.com>
To: Ognyan Kulev <ogi@fmi.uni-sofia.bg>, 190732@bugs.debian.org
Cc: "Alfred M. Szmidt" <ams@kemisten.nu>, metgerards@student.han.nl, marcus.brinkmann@ruhr-uni-bochum.de
Subject: Re: Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
Date: Tue, 2 Mar 2004 21:42:53 -0500 (EST)
> My observation is that such cosmetic changes (like changing comments and 
> memset/memcpy) are rejected by Hurd core developers when they are alone, 
> so the only way to commit them is to merge them with more essential patch.

You are exactly wrong.



Send a report that this bug log contains spam.


Debian bug tracking system administrator <owner@bugs.debian.org>. Last modified: Fri Apr 18 20:48:28 2014; Machine Name: buxtehude.debian.org

Debian Bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.