Skip to content

Commit 6cf3695

Browse files
Tom CherryGerrit Code Review
Tom Cherry
authored and
Gerrit Code Review
committed
Merge "ueventd: add no_fnm_pathname option"
2 parents 0608e36 + 47031c8 commit 6cf3695

6 files changed

+84
-39
lines changed

‎init/README.ueventd.md

+11-3
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ for the node path:
4242
The permissions can be modified using a ueventd.rc script and a line that beings with `/dev`. These
4343
lines take the format of
4444

45-
devname mode uid gid
45+
devname mode uid gid [options]
4646
For example
4747

4848
/dev/null 0666 root root
@@ -80,15 +80,23 @@ Ueventd by default takes no action for `/sys`, however it can be instructed to s
8080
certain files in `/sys` when matching uevents are generated. This is done using a ueventd.rc script
8181
and a line that begins with `/sys`. These lines take the format of
8282

83-
nodename attr mode uid gid
83+
nodename attr mode uid gid [options]
8484
For example
8585

8686
/sys/devices/system/cpu/cpu* cpufreq/scaling_max_freq 0664 system system
8787
When a uevent that matches the pattern `/sys/devices/system/cpu/cpu*` is sent, the matching sysfs
8888
attribute, `cpufreq/scaling_max_freq`, will have its mode set to `0664`, its user to to `system` and
8989
its group set to `system`.
9090

91-
Note that `*` matches as a wildcard and can be used anywhere in a path.
91+
## Path matching
92+
----------------
93+
The path for a `/dev` or `/sys` entry can contain a `*` anywhere in the path.
94+
1. If the only `*` appears at the end of the string or if the _options_ parameter is set to
95+
`no_fnm_pathname`, ueventd matches the entry by `fnmatch(entry_path, incoming_path, 0)`
96+
2. Otherwise, ueventd matches the entry by `fnmatch(entry_path, incoming_path, FNM_PATHNAME)`
97+
98+
See the [man page for fnmatch](https://www.man7.org/linux/man-pages/man3/fnmatch.3.html) for more
99+
details.
92100

93101
## Firmware loading
94102
----------------

‎init/devices.cpp

+11-4
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,15 @@ static bool FindDmDevice(const std::string& path, std::string* name, std::string
124124
return true;
125125
}
126126

127-
Permissions::Permissions(const std::string& name, mode_t perm, uid_t uid, gid_t gid)
128-
: name_(name), perm_(perm), uid_(uid), gid_(gid), prefix_(false), wildcard_(false) {
127+
Permissions::Permissions(const std::string& name, mode_t perm, uid_t uid, gid_t gid,
128+
bool no_fnm_pathname)
129+
: name_(name),
130+
perm_(perm),
131+
uid_(uid),
132+
gid_(gid),
133+
prefix_(false),
134+
wildcard_(false),
135+
no_fnm_pathname_(no_fnm_pathname) {
129136
// Set 'prefix_' or 'wildcard_' based on the below cases:
130137
//
131138
// 1) No '*' in 'name' -> Neither are set and Match() checks a given path for strict
@@ -136,7 +143,6 @@ Permissions::Permissions(const std::string& name, mode_t perm, uid_t uid, gid_t
136143
//
137144
// 3) '*' appears elsewhere -> 'wildcard_' is set to true and Match() uses fnmatch()
138145
// with FNM_PATHNAME to compare 'name' to a given path.
139-
140146
auto wildcard_position = name_.find('*');
141147
if (wildcard_position != std::string::npos) {
142148
if (wildcard_position == name_.length() - 1) {
@@ -150,7 +156,8 @@ Permissions::Permissions(const std::string& name, mode_t perm, uid_t uid, gid_t
150156

151157
bool Permissions::Match(const std::string& path) const {
152158
if (prefix_) return StartsWith(path, name_);
153-
if (wildcard_) return fnmatch(name_.c_str(), path.c_str(), FNM_PATHNAME) == 0;
159+
if (wildcard_)
160+
return fnmatch(name_.c_str(), path.c_str(), no_fnm_pathname_ ? 0 : FNM_PATHNAME) == 0;
154161
return path == name_;
155162
}
156163

‎init/devices.h

+4-3
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class Permissions {
3838
public:
3939
friend void TestPermissions(const Permissions& expected, const Permissions& test);
4040

41-
Permissions(const std::string& name, mode_t perm, uid_t uid, gid_t gid);
41+
Permissions(const std::string& name, mode_t perm, uid_t uid, gid_t gid, bool no_fnm_pathname);
4242

4343
bool Match(const std::string& path) const;
4444

@@ -56,15 +56,16 @@ class Permissions {
5656
gid_t gid_;
5757
bool prefix_;
5858
bool wildcard_;
59+
bool no_fnm_pathname_;
5960
};
6061

6162
class SysfsPermissions : public Permissions {
6263
public:
6364
friend void TestSysfsPermissions(const SysfsPermissions& expected, const SysfsPermissions& test);
6465

6566
SysfsPermissions(const std::string& name, const std::string& attribute, mode_t perm, uid_t uid,
66-
gid_t gid)
67-
: Permissions(name, perm, uid, gid), attribute_(attribute) {}
67+
gid_t gid, bool no_fnm_pathname)
68+
: Permissions(name, perm, uid, gid, no_fnm_pathname), attribute_(attribute) {}
6869

6970
bool MatchWithSubsystem(const std::string& path, const std::string& subsystem) const;
7071
void SetPermissions(const std::string& path) const;

‎init/devices_test.cpp

+26-7
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ TEST(device_handler, sanitize_onebad) {
221221
TEST(device_handler, DevPermissionsMatchNormal) {
222222
// Basic from ueventd.rc
223223
// /dev/null 0666 root root
224-
Permissions permissions("/dev/null", 0666, 0, 0);
224+
Permissions permissions("/dev/null", 0666, 0, 0, false);
225225
EXPECT_TRUE(permissions.Match("/dev/null"));
226226
EXPECT_FALSE(permissions.Match("/dev/nullsuffix"));
227227
EXPECT_FALSE(permissions.Match("/dev/nul"));
@@ -233,7 +233,7 @@ TEST(device_handler, DevPermissionsMatchNormal) {
233233
TEST(device_handler, DevPermissionsMatchPrefix) {
234234
// Prefix from ueventd.rc
235235
// /dev/dri/* 0666 root graphics
236-
Permissions permissions("/dev/dri/*", 0666, 0, 1000);
236+
Permissions permissions("/dev/dri/*", 0666, 0, 1000, false);
237237
EXPECT_TRUE(permissions.Match("/dev/dri/some_dri_device"));
238238
EXPECT_TRUE(permissions.Match("/dev/dri/some_other_dri_device"));
239239
EXPECT_TRUE(permissions.Match("/dev/dri/"));
@@ -246,7 +246,7 @@ TEST(device_handler, DevPermissionsMatchPrefix) {
246246
TEST(device_handler, DevPermissionsMatchWildcard) {
247247
// Wildcard example
248248
// /dev/device*name 0666 root graphics
249-
Permissions permissions("/dev/device*name", 0666, 0, 1000);
249+
Permissions permissions("/dev/device*name", 0666, 0, 1000, false);
250250
EXPECT_TRUE(permissions.Match("/dev/devicename"));
251251
EXPECT_TRUE(permissions.Match("/dev/device123name"));
252252
EXPECT_TRUE(permissions.Match("/dev/deviceabcname"));
@@ -260,13 +260,31 @@ TEST(device_handler, DevPermissionsMatchWildcard) {
260260
TEST(device_handler, DevPermissionsMatchWildcardPrefix) {
261261
// Wildcard+Prefix example
262262
// /dev/device*name* 0666 root graphics
263-
Permissions permissions("/dev/device*name*", 0666, 0, 1000);
263+
Permissions permissions("/dev/device*name*", 0666, 0, 1000, false);
264264
EXPECT_TRUE(permissions.Match("/dev/devicename"));
265265
EXPECT_TRUE(permissions.Match("/dev/device123name"));
266266
EXPECT_TRUE(permissions.Match("/dev/deviceabcname"));
267267
EXPECT_TRUE(permissions.Match("/dev/device123namesomething"));
268268
// FNM_PATHNAME doesn't match '/' with *
269269
EXPECT_FALSE(permissions.Match("/dev/device123name/something"));
270+
EXPECT_FALSE(permissions.Match("/dev/device/1/2/3name/something"));
271+
EXPECT_FALSE(permissions.Match("/dev/deviceame"));
272+
EXPECT_EQ(0666U, permissions.perm());
273+
EXPECT_EQ(0U, permissions.uid());
274+
EXPECT_EQ(1000U, permissions.gid());
275+
}
276+
277+
TEST(device_handler, DevPermissionsMatchWildcardPrefix_NoFnmPathName) {
278+
// Wildcard+Prefix example with no_fnm_pathname
279+
// /dev/device*name* 0666 root graphics
280+
Permissions permissions("/dev/device*name*", 0666, 0, 1000, true);
281+
EXPECT_TRUE(permissions.Match("/dev/devicename"));
282+
EXPECT_TRUE(permissions.Match("/dev/device123name"));
283+
EXPECT_TRUE(permissions.Match("/dev/deviceabcname"));
284+
EXPECT_TRUE(permissions.Match("/dev/device123namesomething"));
285+
// With NoFnmPathName, the below matches, unlike DevPermissionsMatchWildcardPrefix.
286+
EXPECT_TRUE(permissions.Match("/dev/device123name/something"));
287+
EXPECT_TRUE(permissions.Match("/dev/device/1/2/3name/something"));
270288
EXPECT_FALSE(permissions.Match("/dev/deviceame"));
271289
EXPECT_EQ(0666U, permissions.perm());
272290
EXPECT_EQ(0U, permissions.uid());
@@ -275,7 +293,8 @@ TEST(device_handler, DevPermissionsMatchWildcardPrefix) {
275293

276294
TEST(device_handler, SysfsPermissionsMatchWithSubsystemNormal) {
277295
// /sys/devices/virtual/input/input* enable 0660 root input
278-
SysfsPermissions permissions("/sys/devices/virtual/input/input*", "enable", 0660, 0, 1001);
296+
SysfsPermissions permissions("/sys/devices/virtual/input/input*", "enable", 0660, 0, 1001,
297+
false);
279298
EXPECT_TRUE(permissions.MatchWithSubsystem("/sys/devices/virtual/input/input0", "input"));
280299
EXPECT_FALSE(permissions.MatchWithSubsystem("/sys/devices/virtual/input/not_input0", "input"));
281300
EXPECT_EQ(0660U, permissions.perm());
@@ -285,7 +304,7 @@ TEST(device_handler, SysfsPermissionsMatchWithSubsystemNormal) {
285304

286305
TEST(device_handler, SysfsPermissionsMatchWithSubsystemClass) {
287306
// /sys/class/input/event* enable 0660 root input
288-
SysfsPermissions permissions("/sys/class/input/event*", "enable", 0660, 0, 1001);
307+
SysfsPermissions permissions("/sys/class/input/event*", "enable", 0660, 0, 1001, false);
289308
EXPECT_TRUE(permissions.MatchWithSubsystem(
290309
"/sys/devices/soc.0/f9924000.i2c/i2c-2/2-0020/input/input0/event0", "input"));
291310
EXPECT_FALSE(permissions.MatchWithSubsystem(
@@ -299,7 +318,7 @@ TEST(device_handler, SysfsPermissionsMatchWithSubsystemClass) {
299318

300319
TEST(device_handler, SysfsPermissionsMatchWithSubsystemBus) {
301320
// /sys/bus/i2c/devices/i2c-* enable 0660 root input
302-
SysfsPermissions permissions("/sys/bus/i2c/devices/i2c-*", "enable", 0660, 0, 1001);
321+
SysfsPermissions permissions("/sys/bus/i2c/devices/i2c-*", "enable", 0660, 0, 1001, false);
303322
EXPECT_TRUE(permissions.MatchWithSubsystem("/sys/devices/soc.0/f9967000.i2c/i2c-5", "i2c"));
304323
EXPECT_FALSE(permissions.MatchWithSubsystem("/sys/devices/soc.0/f9967000.i2c/not-i2c", "i2c"));
305324
EXPECT_FALSE(

‎init/ueventd_parser.cpp

+15-6
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,12 @@ Result<void> ParsePermissionsLine(std::vector<std::string>&& args,
3434
std::vector<SysfsPermissions>* out_sysfs_permissions,
3535
std::vector<Permissions>* out_dev_permissions) {
3636
bool is_sysfs = out_sysfs_permissions != nullptr;
37-
if (is_sysfs && args.size() != 5) {
38-
return Error() << "/sys/ lines must have 5 entries";
37+
if (is_sysfs && !(args.size() == 5 || args.size() == 6)) {
38+
return Error() << "/sys/ lines must have 5 or 6 entries";
3939
}
4040

41-
if (!is_sysfs && args.size() != 4) {
42-
return Error() << "/dev/ lines must have 4 entries";
41+
if (!is_sysfs && !(args.size() == 4 || args.size() == 5)) {
42+
return Error() << "/dev/ lines must have 4 or 5 entries";
4343
}
4444

4545
auto it = args.begin();
@@ -70,10 +70,19 @@ Result<void> ParsePermissionsLine(std::vector<std::string>&& args,
7070
}
7171
gid_t gid = grp->gr_gid;
7272

73+
bool no_fnm_pathname = false;
74+
if (it != args.end()) {
75+
std::string& flags = *it++;
76+
if (flags != "no_fnm_pathname") {
77+
return Error() << "invalid option '" << flags << "', only no_fnm_pathname is supported";
78+
}
79+
no_fnm_pathname = true;
80+
}
81+
7382
if (is_sysfs) {
74-
out_sysfs_permissions->emplace_back(name, sysfs_attribute, perm, uid, gid);
83+
out_sysfs_permissions->emplace_back(name, sysfs_attribute, perm, uid, gid, no_fnm_pathname);
7584
} else {
76-
out_dev_permissions->emplace_back(name, perm, uid, gid);
85+
out_dev_permissions->emplace_back(name, perm, uid, gid, no_fnm_pathname);
7786
}
7887
return {};
7988
}

‎init/ueventd_parser_test.cpp

+17-16
Original file line numberDiff line numberDiff line change
@@ -104,21 +104,21 @@ TEST(ueventd_parser, Permissions) {
104104
/dev/graphics/* 0660 root graphics
105105
/dev/*/test 0660 root system
106106
107-
/sys/devices/platform/trusty.* trusty_version 0440 root log
108-
/sys/devices/virtual/input/input enable 0660 root input
109-
/sys/devices/virtual/*/input poll_delay 0660 root input
107+
/sys/devices/platform/trusty.* trusty_version 0440 root log
108+
/sys/devices/virtual/input/input enable 0660 root input
109+
/sys/devices/virtual/*/input poll_delay 0660 root input no_fnm_pathname
110110
)";
111111

112112
auto permissions = std::vector<Permissions>{
113-
{"/dev/rtc0", 0640, AID_SYSTEM, AID_SYSTEM},
114-
{"/dev/graphics/*", 0660, AID_ROOT, AID_GRAPHICS},
115-
{"/dev/*/test", 0660, AID_ROOT, AID_SYSTEM},
113+
{"/dev/rtc0", 0640, AID_SYSTEM, AID_SYSTEM, false},
114+
{"/dev/graphics/*", 0660, AID_ROOT, AID_GRAPHICS, false},
115+
{"/dev/*/test", 0660, AID_ROOT, AID_SYSTEM, false},
116116
};
117117

118118
auto sysfs_permissions = std::vector<SysfsPermissions>{
119-
{"/sys/devices/platform/trusty.*", "trusty_version", 0440, AID_ROOT, AID_LOG},
120-
{"/sys/devices/virtual/input/input", "enable", 0660, AID_ROOT, AID_INPUT},
121-
{"/sys/devices/virtual/*/input", "poll_delay", 0660, AID_ROOT, AID_INPUT},
119+
{"/sys/devices/platform/trusty.*", "trusty_version", 0440, AID_ROOT, AID_LOG, false},
120+
{"/sys/devices/virtual/input/input", "enable", 0660, AID_ROOT, AID_INPUT, false},
121+
{"/sys/devices/virtual/*/input", "poll_delay", 0660, AID_ROOT, AID_INPUT, true},
122122
};
123123

124124
TestUeventdFile(ueventd_file, {{}, sysfs_permissions, permissions, {}, {}});
@@ -240,7 +240,7 @@ subsystem test_devpath_dirname
240240
dirname /dev/graphics
241241
242242
/dev/*/test 0660 root system
243-
/sys/devices/virtual/*/input poll_delay 0660 root input
243+
/sys/devices/virtual/*/input poll_delay 0660 root input no_fnm_pathname
244244
firmware_directories /more
245245
246246
external_firmware_handler /devices/path/firmware/firmware001.bin root /vendor/bin/touch.sh
@@ -259,15 +259,15 @@ parallel_restorecon enabled
259259
{"test_devpath_dirname", Subsystem::DEVNAME_UEVENT_DEVPATH, "/dev/graphics"}};
260260

261261
auto permissions = std::vector<Permissions>{
262-
{"/dev/rtc0", 0640, AID_SYSTEM, AID_SYSTEM},
263-
{"/dev/graphics/*", 0660, AID_ROOT, AID_GRAPHICS},
264-
{"/dev/*/test", 0660, AID_ROOT, AID_SYSTEM},
262+
{"/dev/rtc0", 0640, AID_SYSTEM, AID_SYSTEM, false},
263+
{"/dev/graphics/*", 0660, AID_ROOT, AID_GRAPHICS, false},
264+
{"/dev/*/test", 0660, AID_ROOT, AID_SYSTEM, false},
265265
};
266266

267267
auto sysfs_permissions = std::vector<SysfsPermissions>{
268-
{"/sys/devices/platform/trusty.*", "trusty_version", 0440, AID_ROOT, AID_LOG},
269-
{"/sys/devices/virtual/input/input", "enable", 0660, AID_ROOT, AID_INPUT},
270-
{"/sys/devices/virtual/*/input", "poll_delay", 0660, AID_ROOT, AID_INPUT},
268+
{"/sys/devices/platform/trusty.*", "trusty_version", 0440, AID_ROOT, AID_LOG, false},
269+
{"/sys/devices/virtual/input/input", "enable", 0660, AID_ROOT, AID_INPUT, false},
270+
{"/sys/devices/virtual/*/input", "poll_delay", 0660, AID_ROOT, AID_INPUT, true},
271271
};
272272

273273
auto firmware_directories = std::vector<std::string>{
@@ -299,6 +299,7 @@ firmware_directories #no directory listed
299299
/sys/devices/platform/trusty.* trusty_version badmode root log
300300
/sys/devices/platform/trusty.* trusty_version 0440 baduidbad log
301301
/sys/devices/platform/trusty.* trusty_version 0440 root baduidbad
302+
/sys/devices/platform/trusty.* trusty_version 0440 root root bad_option
302303
303304
uevent_socket_rcvbuf_size blah
304305

0 commit comments

Comments
 (0)