Re: [PATCH 1/6] pci-arbiter: Fix brokenness

[Samuel Thibault]

Damien Zammit, le dim. 07 mars 2021 17:39:05 +1100, a ecrit:
> -  if (max_entries < 0)
> -    count = dir->dir->num_entries;

> +  count = nentries - first_entry;
> +  if (max_entries > 0 && count > max_entries)
> +    count = max_entries;

It was previously considering max_entries == 0 meaning really zero, so
better keep it that way by testing for max_entries >= 0.

> -  size =
> -    (count * DIRENTS_CHUNK_SIZE) >
> -    max_data_len ? max_data_len : count * DIRENTS_CHUNK_SIZE;
> +  size = count * DIRENTS_CHUNK_SIZE;

You need to keep taking max_data_len into account.

Also, the exact same code is appearing in acpi/netfs_impl.c, so please
apply the same fixes there, otherwise we'll keep that max_entries bug
along with acpi (and who knows when that will be cargo-culted again).

> @@ -234,8 +228,8 @@ netfs_get_dirents (struct iouser * cred, struct node * 
> dir,
>  
>    if (dir->nn->ln->dir)
>      {
> -      err = get_dirents (dir->nn->ln, first_entry, max_entries,
> -                      data, data_len, max_entries, data_entries);
> +      err = get_dirents (dir->nn->ln, first_entry, -1,
> +                      data, data_len, 0, data_entries);

Keep max_entries as such, the RPC caller could be very surprised to get
more entries than what it asked for.

> @@ -257,6 +251,24 @@ netfs_attempt_lookup (struct iouser * user, struct node 
> * dir,
>  {
>    error_t err = 0;
>    struct pcifs_dirent *entry;
> +  char *last = name;
> +
> +  /* Strip trailing slashes */
> +  if (*last)
> +    {
> +      while (*last)
> +        last++;
> +      last--;

Rather use last += strlen(last)-1, strlen is optimized by glibc.

> +      while (*last == '/' && last >= name)
> +        {
> +          *last = '\0';
> +          last--;

It should also record in a mustbedir boolean that there was a trailing
slash. A trailing slash, in Posix, means that the caller really wanted
to lookup a directory, so the flag should passed to the lookup()
function above to check that IFTODT (e->stat.st_mode) == DT_DIR, and
otherwise return an ENOTDIR error.

Samuel