1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
|
From c0899e5b913b70f40b4ac4a314d18437f82ad5bc Mon Sep 17 00:00:00 2001
From: Panu Matilainen <pmatilai@redhat.com>
Date: Mon, 14 Mar 2022 09:18:28 +0200
Subject: [PATCH 1/2] Fix file descriptor leak regression on install (#1947)
Commit 0e3024ca3e7450104e70ec8d213cf223e71f7c02 introduced a leak on
directory file descriptors from hardlinked sets, preventing some large
packages with many hardlinks from installing at all.
fsmMkfile() needs to close the firstdir fd when done with it because
that's the only place that knows when it's safe to do so. However, there
could be non-hardlink entries left in the same directory, so we must not
close *that* descriptor. Dup the firstdir descriptor so we're free to
close it without worrying about the other state.
Fixes: #1947
---
lib/fsm.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/lib/fsm.c b/lib/fsm.c
index 4859d62ee6..5a74b43cd7 100644
--- a/lib/fsm.c
+++ b/lib/fsm.c
@@ -208,7 +208,7 @@ static int fsmMkfile(int dirfd, rpmfi fi, struct filedata_s *fp, rpmfiles files,
if (fp->sb.st_nlink > 1) {
*firstlink = fp;
*firstlinkfile = fd;
- *firstdir = dirfd;
+ *firstdir = dup(dirfd);
}
} else {
/* Create hard links for others and avoid redundant metadata setting */
@@ -227,7 +227,7 @@ static int fsmMkfile(int dirfd, rpmfi fi, struct filedata_s *fp, rpmfiles files,
fp->setmeta = 1;
*firstlink = NULL;
*firstlinkfile = -1;
- *firstdir = -1;
+ fsmClose(firstdir);
}
}
*fdp = fd;
@@ -841,8 +841,7 @@ static int onChdir(rpmfi fi, void *data)
struct diriter_s *di = data;
if (di->dirfd >= 0) {
- if (di->dirfd != di->firstdir)
- close(di->dirfd);
+ close(di->dirfd);
di->dirfd = -1;
}
return 0;
From 661a19f08f07fdf1e4b246f8d8bbcfc193531033 Mon Sep 17 00:00:00 2001
From: Panu Matilainen <pmatilai@redhat.com>
Date: Mon, 14 Mar 2022 09:52:21 +0200
Subject: [PATCH 2/2] Use fsmClose() for closing file descriptors everywhere
within fsm
fsmClose() does all the necessary checks and resets the fd to -1 after
close, why duplicate these all over the code when we already have a
function...
There's no difference wrt file descriptors getting closed here, but
there is a side-effect to this: if %_flush_io is enabled, they now
get fsync() on the associated directories too, which I think is a good
thing for those who care about it.
---
lib/fsm.c | 28 +++++++++-------------------
1 file changed, 9 insertions(+), 19 deletions(-)
diff --git a/lib/fsm.c b/lib/fsm.c
index 5a74b43cd7..28eb9431bd 100644
--- a/lib/fsm.c
+++ b/lib/fsm.c
@@ -66,6 +66,7 @@ struct filedata_s {
*/
static const char * fileActionString(rpmFileAction a);
static int fsmOpenat(int dirfd, const char *path, int flags, int dir);
+static int fsmClose(int *wfdp);
/** \ingroup payload
* Build path to file from file info, optionally ornamented with suffix.
@@ -101,7 +102,7 @@ static int cap_set_fileat(int dirfd, const char *path, cap_t fcaps)
int fd = fsmOpenat(dirfd, path, O_RDONLY|O_NOFOLLOW, 0);
if (fd >= 0) {
rc = cap_set_fd(fd, fcaps);
- close(fd);
+ fsmClose(&fd);
}
return rc;
}
@@ -326,8 +327,7 @@ static int fsmOpenat(int dirfd, const char *path, int flags, int dir)
/* O_DIRECTORY equivalent */
if (dir && fd >= 0 && fstat(fd, &sb) == 0 && !S_ISDIR(sb.st_mode)) {
errno = ENOTDIR;
- close(fd);
- fd = -1;
+ fsmClose(&fd);
}
return fd;
}
@@ -395,7 +395,7 @@ static int ensureDir(rpmPlugins plugins, const char *p, int owned, int create,
rc = fsmDoMkDir(plugins, dirfd, bn, apath, owned, mode, &fd);
}
- close(dirfd);
+ fsmClose(&dirfd);
if (fd >= 0) {
dirfd = fd;
} else {
@@ -411,9 +411,8 @@ static int ensureDir(rpmPlugins plugins, const char *p, int owned, int create,
}
if (rc) {
- close(fd);
- close(dirfd);
- dirfd = -1;
+ fsmClose(&fd);
+ fsmClose(&dirfd);
} else {
rc = 0;
}
@@ -840,10 +839,7 @@ static int onChdir(rpmfi fi, void *data)
{
struct diriter_s *di = data;
- if (di->dirfd >= 0) {
- close(di->dirfd);
- di->dirfd = -1;
- }
+ fsmClose(&(di->dirfd));
return 0;
}
@@ -861,14 +857,8 @@ static rpmfi fsmIter(FD_t payload, rpmfiles files, rpmFileIter iter, void *data)
static rpmfi fsmIterFini(rpmfi fi, struct diriter_s *di)
{
- if (di->dirfd >= 0) {
- close(di->dirfd);
- di->dirfd = -1;
- }
- if (di->firstdir >= 0) {
- close(di->firstdir);
- di->firstdir = -1;
- }
+ fsmClose(&(di->dirfd));
+ fsmClose(&(di->firstdir));
return rpmfiFree(fi);
}
|
