correct readlinkat attribute access (BZ #27024)

Message ID b39497cd-12ae-981c-73d4-22e99d57ba77@gmail.com
State Committed
Headers
Series correct readlinkat attribute access (BZ #27024) |

Commit Message

Martin Sebor Jan. 8, 2021, 12:49 a.m. UTC
  The mode in the new attribute access added in g:06febd8c670
to readlinkat() for the third argument is wrong: it's read_only
but the function writes into the object.  The mode should be
write_only.

The trivial patch below corrects this mistake.  Unless there
are objections I'll go ahead and commit it in the next few days.

Martin


  /* Remove the link NAME.  */
  

Comments

Siddhesh Poyarekar Jan. 8, 2021, 4:31 a.m. UTC | #1
On 1/8/21 6:19 AM, Martin Sebor via Libc-alpha wrote:
> The mode in the new attribute access added in g:06febd8c670
> to readlinkat() for the third argument is wrong: it's read_only
> but the function writes into the object.  The mode should be
> write_only.
> 
> The trivial patch below corrects this mistake.  Unless there
> are objections I'll go ahead and commit it in the next few days.
> 
> Martin
> 
> diff --git a/posix/unistd.h b/posix/unistd.h
> index 1306aded46..3f22763379 100644
> --- a/posix/unistd.h
> +++ b/posix/unistd.h
> @@ -831,7 +831,7 @@ extern int symlinkat (const char *__from, int __tofd,
>   /* Like readlink but a relative PATH is interpreted relative to FD.  */
>   extern ssize_t readlinkat (int __fd, const char *__restrict __path,
>                             char *__restrict __buf, size_t __len)
> -     __THROW __nonnull ((2, 3)) __wur __attr_access ((__read_only__, 3, 
> 4));
> +     __THROW __nonnull ((2, 3)) __wur __attr_access ((__write_only__, 
> 3, 4));
>   #endif

Why do we need an access attribute at all for __len, let alone a 
__write_only__ one?

Siddhesh
  
Samuel Thibault Jan. 8, 2021, 9:06 a.m. UTC | #2
Siddhesh Poyarekar, le ven. 08 janv. 2021 10:01:22 +0530, a ecrit:
> On 1/8/21 6:19 AM, Martin Sebor via Libc-alpha wrote:
> > @@ -831,7 +831,7 @@ extern int symlinkat (const char *__from, int __tofd,
> >   /* Like readlink but a relative PATH is interpreted relative to FD.  */
> >   extern ssize_t readlinkat (int __fd, const char *__restrict __path,
> >                             char *__restrict __buf, size_t __len)
> > -     __THROW __nonnull ((2, 3)) __wur __attr_access ((__read_only__, 3,
> > 4));
> > +     __THROW __nonnull ((2, 3)) __wur __attr_access ((__write_only__,
> > 3, 4));
> >   #endif
> 
> Why do we need an access attribute at all for __len, let alone a
> __write_only__ one?

? This is not on __len, this is on __buf (arg 3) for __len bytes (arg 4)

Samuel
  
Siddhesh Poyarekar Jan. 8, 2021, 9:08 a.m. UTC | #3
On 1/8/21 2:36 PM, Samuel Thibault wrote:
>> Why do we need an access attribute at all for __len, let alone a
>> __write_only__ one?
> 
> ? This is not on __len, this is on __buf (arg 3) for __len bytes (arg 4)
> 

Ugh, sorry I misread that.

Siddhesh
  
Martin Sebor Jan. 10, 2021, 8:52 p.m. UTC | #4
I committed & pushed this in g:2cd361b511.

Martin

On 1/7/21 5:49 PM, Martin Sebor wrote:
> The mode in the new attribute access added in g:06febd8c670
> to readlinkat() for the third argument is wrong: it's read_only
> but the function writes into the object.  The mode should be
> write_only.
> 
> The trivial patch below corrects this mistake.  Unless there
> are objections I'll go ahead and commit it in the next few days.
> 
> Martin
> 
> diff --git a/posix/unistd.h b/posix/unistd.h
> index 1306aded46..3f22763379 100644
> --- a/posix/unistd.h
> +++ b/posix/unistd.h
> @@ -831,7 +831,7 @@ extern int symlinkat (const char *__from, int __tofd,
>   /* Like readlink but a relative PATH is interpreted relative to FD.  */
>   extern ssize_t readlinkat (int __fd, const char *__restrict __path,
>                             char *__restrict __buf, size_t __len)
> -     __THROW __nonnull ((2, 3)) __wur __attr_access ((__read_only__, 3, 
> 4));
> +     __THROW __nonnull ((2, 3)) __wur __attr_access ((__write_only__, 
> 3, 4));
>   #endif
> 
>   /* Remove the link NAME.  */
  

Patch

diff --git a/posix/unistd.h b/posix/unistd.h
index 1306aded46..3f22763379 100644
--- a/posix/unistd.h
+++ b/posix/unistd.h
@@ -831,7 +831,7 @@  extern int symlinkat (const char *__from, int __tofd,
  /* Like readlink but a relative PATH is interpreted relative to FD.  */
  extern ssize_t readlinkat (int __fd, const char *__restrict __path,
                            char *__restrict __buf, size_t __len)
-     __THROW __nonnull ((2, 3)) __wur __attr_access ((__read_only__, 3, 
4));
+     __THROW __nonnull ((2, 3)) __wur __attr_access ((__write_only__, 
3, 4));
  #endif