Hi David,
David Howells <dhowells@xxxxxxxxxx> writes:
> Your patch needs a bit of adjustment. The check must be done inside the write
> lock on afs_cells_sem otherwise there's a race.
>
> David
> ---
> AFS: prevent double cell registration
>
> From: Sven Schnelle <svens@xxxxxxxxxxxxxx>
>
> kafs doesn't check if the cell already exists - so if you do an
> echo "add newcell.org 1.2.3.4" >/proc/fs/afs/cells it will try to
> create this cell again. kobject will also complain about a double
> registration. To prevent such problems, return -EEXIST in that case.
>
> Signed-off-by: Sven Schnelle <svens@xxxxxxxxxxxxxx>
> Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> ---
>
> fs/afs/cell.c | 15 +++++++++++++--
> 1 files changed, 13 insertions(+), 2 deletions(-)
>
>
> diff --git a/fs/afs/cell.c b/fs/afs/cell.c
> index 970d38f..788865d 100644
> --- a/fs/afs/cell.c
> +++ b/fs/afs/cell.c
> @@ -127,14 +127,20 @@ struct afs_cell *afs_cell_create(const char *name, char *vllist)
>
> _enter("%s,%s", name, vllist);
>
> + down_write(&afs_cells_sem);
> + list_for_each_entry(cell, &afs_cells, link) {
> + if (strcasecmp(cell->name, name) == 0)
> + goto duplicate_name;
> + }
> + read_unlock(&afs_cells_lock);
> +
> cell = afs_cell_alloc(name, vllist);
> if (IS_ERR(cell)) {
Just digged again into afs, and stumbled upon this return. I think there should
be some up_write(&afs_cells_sem); before the return, or am i on the wrong track?
> _leave(" = %ld", PTR_ERR(cell));
> return cell;
> }
>
> - down_write(&afs_cells_sem);
> -
> /* add a proc directory for this cell */
> ret = afs_proc_cell_setup(cell);
> if (ret < 0)
> @@ -167,6 +173,11 @@ error:
> kfree(cell);
> _leave(" = %d", ret);
> return ERR_PTR(ret);
> +
> +duplicate_name:
> + read_unlock(&afs_cells_lock);
> + up_write(&afs_cells_sem);
> + return ERR_PTR(-EEXIST);
> }
>
> /*
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
