| Message ID | 20210114155920.293559-1-plautrba@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] libselinux: Always close status page fd | expand |
On Thu, Jan 14, 2021 at 10:02 AM Petr Lautrbach <plautrba@redhat.com> wrote: > > According to mmap(2) after the mmap() call has returned, the file > descriptor, fd, can be closed immediately without invalidating the > mapping. > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com> > --- > > Changes against v1: > - selinux_status_fd is completely droped as it's actually unused > > libselinux/src/sestatus.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/libselinux/src/sestatus.c b/libselinux/src/sestatus.c > index 9ff2785d876a..12b015e088ea 100644 > --- a/libselinux/src/sestatus.c > +++ b/libselinux/src/sestatus.c > @@ -37,7 +37,6 @@ struct selinux_status_t > * Valid Pointer : opened and mapped correctly > */ > static struct selinux_status_t *selinux_status = NULL; > -static int selinux_status_fd; > static uint32_t last_seqno; > static uint32_t last_policyload; > > @@ -298,11 +297,10 @@ int selinux_status_open(int fallback) > goto error; > > selinux_status = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, fd, 0); > + close(fd); > if (selinux_status == MAP_FAILED) { > - close(fd); > goto error; > } > - selinux_status_fd = fd; > last_seqno = (uint32_t)(-1); > > /* sequence must not be changed during references */ > @@ -336,7 +334,6 @@ error: > > /* mark as fallback mode */ > selinux_status = MAP_FAILED; > - selinux_status_fd = avc_netlink_acquire_fd(); > last_seqno = (uint32_t)(-1); > > if (avc_using_threads) > @@ -388,7 +385,5 @@ void selinux_status_close(void) > munmap(selinux_status, pagesize); > selinux_status = NULL; > > - close(selinux_status_fd); > - selinux_status_fd = -1; > last_seqno = (uint32_t)(-1); > } > -- > 2.30.0 > ack and staged. I love a negative diffstat. Thanks Petr. https://github.com/SELinuxProject/selinux/pull/279
On Thu, Jan 14, 2021 at 10:50 AM William Roberts <bill.c.roberts@gmail.com> wrote: > > On Thu, Jan 14, 2021 at 10:02 AM Petr Lautrbach <plautrba@redhat.com> wrote: > > > > According to mmap(2) after the mmap() call has returned, the file > > descriptor, fd, can be closed immediately without invalidating the > > mapping. > > > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com> > > --- > > > > Changes against v1: > > - selinux_status_fd is completely droped as it's actually unused > > > > libselinux/src/sestatus.c | 7 +------ > > 1 file changed, 1 insertion(+), 6 deletions(-) > > > > diff --git a/libselinux/src/sestatus.c b/libselinux/src/sestatus.c > > index 9ff2785d876a..12b015e088ea 100644 > > --- a/libselinux/src/sestatus.c > > +++ b/libselinux/src/sestatus.c > > @@ -37,7 +37,6 @@ struct selinux_status_t > > * Valid Pointer : opened and mapped correctly > > */ > > static struct selinux_status_t *selinux_status = NULL; > > -static int selinux_status_fd; > > static uint32_t last_seqno; > > static uint32_t last_policyload; > > > > @@ -298,11 +297,10 @@ int selinux_status_open(int fallback) > > goto error; > > > > selinux_status = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, fd, 0); > > + close(fd); > > if (selinux_status == MAP_FAILED) { > > - close(fd); > > goto error; > > } > > - selinux_status_fd = fd; > > last_seqno = (uint32_t)(-1); > > > > /* sequence must not be changed during references */ > > @@ -336,7 +334,6 @@ error: > > > > /* mark as fallback mode */ > > selinux_status = MAP_FAILED; > > - selinux_status_fd = avc_netlink_acquire_fd(); > > last_seqno = (uint32_t)(-1); > > > > if (avc_using_threads) > > @@ -388,7 +385,5 @@ void selinux_status_close(void) > > munmap(selinux_status, pagesize); > > selinux_status = NULL; > > > > - close(selinux_status_fd); > > - selinux_status_fd = -1; > > last_seqno = (uint32_t)(-1); > > } > > -- > > 2.30.0 > > > > ack and staged. I love a negative diffstat. Thanks Petr. > https://github.com/SELinuxProject/selinux/pull/279 merged
diff --git a/libselinux/src/sestatus.c b/libselinux/src/sestatus.c index 9ff2785d876a..12b015e088ea 100644 --- a/libselinux/src/sestatus.c +++ b/libselinux/src/sestatus.c @@ -37,7 +37,6 @@ struct selinux_status_t * Valid Pointer : opened and mapped correctly */ static struct selinux_status_t *selinux_status = NULL; -static int selinux_status_fd; static uint32_t last_seqno; static uint32_t last_policyload; @@ -298,11 +297,10 @@ int selinux_status_open(int fallback) goto error; selinux_status = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, fd, 0); + close(fd); if (selinux_status == MAP_FAILED) { - close(fd); goto error; } - selinux_status_fd = fd; last_seqno = (uint32_t)(-1); /* sequence must not be changed during references */ @@ -336,7 +334,6 @@ error: /* mark as fallback mode */ selinux_status = MAP_FAILED; - selinux_status_fd = avc_netlink_acquire_fd(); last_seqno = (uint32_t)(-1); if (avc_using_threads) @@ -388,7 +385,5 @@ void selinux_status_close(void) munmap(selinux_status, pagesize); selinux_status = NULL; - close(selinux_status_fd); - selinux_status_fd = -1; last_seqno = (uint32_t)(-1); }
According to mmap(2) after the mmap() call has returned, the file descriptor, fd, can be closed immediately without invalidating the mapping. Signed-off-by: Petr Lautrbach <plautrba@redhat.com> --- Changes against v1: - selinux_status_fd is completely droped as it's actually unused libselinux/src/sestatus.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)