[Bug libc/29417] New: poll() declared with write-only fds

[Bug libc/29417] New: poll() declared with write-only fds

[mlichvar at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29417

            Bug ID: 29417
           Summary: poll() declared with write-only fds
           Product: glibc
           Version: 2.36
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
          Assignee: unassigned at sourceware dot org
          Reporter: mlichvar at redhat dot com
                CC: drepper.fsp at gmail dot com
  Target Milestone: ---

The poll() function is currently declared as

extern int poll (struct pollfd *__fds, nfds_t __nfds, int __timeout)
    __fortified_attr_access (__write_only__, 1, 2);

Is the write-only access of __fds correct? I think the poll function needs to
read the fd and events field of pollfd.

Maybe I don't understand correctly how it's supposed to work, but the access
specified for some other functions made sense to me.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29417] poll() declared with write-only fds

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29417

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-
                 CC|                            |fweimer at redhat dot com,
                   |                            |siddhesh at sourceware dot org

--- Comment #1 from Florian Weimer <fweimer at redhat dot com> ---
I believe write_only is correct here because read_write would require full
initialization of all struct pollfd members. According to the poll manual page,
the revents member does not need to be initialized because the kernel
overwrites it.

Siddhesh, what do you think?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29417] poll() declared with write-only fds

[mlichvar at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29417

--- Comment #2 from Miroslav Lichvar <mlichvar at redhat dot com> ---
FWIW, a poll() function using that declaration triggers a compiler warning:

#include <poll.h>
int poll(struct pollfd *fds, nfds_t nfds, int timeout) {
        return nfds >= 1 && fds[0].fd >= 0;
}

x.c: In function ‘poll’:
x.c:4:35: warning: ‘*fds.fd’ may be used uninitialized [-Wmaybe-uninitialized]
    4 |         return nfds >= 1 && fds[0].fd >= 0;
      |                             ~~~~~~^~~
x.c:3:25: note: accessing argument 1 of a function declared with attribute
‘access (write_only, 1, 2)’
    3 | int poll(struct pollfd *fds, nfds_t nfds, int timeout) {
      |          ~~~~~~~~~~~~~~~^~~

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29417] poll() declared with write-only fds

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29417

--- Comment #3 from Florian Weimer <fweimer at redhat dot com> ---
It's a warning in the implementation, and that doesn't matter to us.

What would be problematic if GCC decided to eliminate stores to the fds array,
assuming that it will be overwritten completely. But I don't think the
specification for write_only allows that. (write_only is perhaps misnamed.)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29417] poll() declared with write-only fds

[siddhesh at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29417

--- Comment #4 from Siddhesh Poyarekar <siddhesh at sourceware dot org> ---
(In reply to Florian Weimer from comment #3)
> What would be problematic if GCC decided to eliminate stores to the fds
> array, assuming that it will be overwritten completely. But I don't think
> the specification for write_only allows that. (write_only is perhaps
> misnamed.)

The compiler cannot eliminate stores because it doesn't know if poll will
overwrite the object.  That said, I wouldn't stick out my neck and say that the
write_only attribute is absolutely correct in this context; the structure does
need to be read further down ithe stack n the syscall.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29417] poll() declared with write-only fds

[siddhesh at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29417

--- Comment #5 from Siddhesh Poyarekar <siddhesh at sourceware dot org> ---
Sorry, hit send a bit too soon.  read_write is also not appropriate for reasons
Florian mentioned.  So to conclude, while the strictly correct option here is
to not have any annotations, it will also get rid of object size related
protection, which is a worse place to be in IMO.

So IMO, the current state of this declaration is the most useful.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29417] poll() declared with write-only fds

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29417

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |NOTABUG
             Status|NEW                         |RESOLVED

--- Comment #6 from Florian Weimer <fweimer at redhat dot com> ---
Thanks for your comments, Siddhesh? Would it make sense to enhance the GCC
documentation? If only to start a discussion among GCC developers what these
attributes mean, exactly. 8-)

-- 
You are receiving this mail because:
You are on the CC list for the bug.