* [tarantool-patches] [PATCH] lua: pwd: fix passwd and group traversal
@ 2019-08-23 1:57 Alexander Turenko
2019-08-23 20:26 ` [tarantool-patches] " Vladislav Shpilevoy
2019-08-26 15:08 ` Kirill Yukhin
0 siblings, 2 replies; 6+ messages in thread
From: Alexander Turenko @ 2019-08-23 1:57 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: Alexander Turenko, tarantool-patches
CentOS 6 and FreeBSD 12 implementations of getpwuid() rewind
setpwent()-{getpwent()}-endpwent() loop to a start that leads to a hang
during pwd.getpwall() invoke. The same is true for a getgrgid() call
during setgrent()-{getgrent()}-endgrent() loop.
The commit modifies pwd module to avoid getpwuid() calls during passwd
database traversal and to avoid getgrgid() calls when traversing groups.
The commit also fixes the important regression on CentOS 6 after
f5d8331e833d6a29dc7a869e681e03fb8e03a5f8 ('lua: workaround
pwd.getpwall() issue on Fedora 29'): tarantool hungs during startup due
to added getpwall() call. This made tarantool unusable on CentOS 6 at
all.
Aside of that the commit fixes another pwd.getgrall() problem: the
function gaves password entries instead of group entries.
Fixes #4428.
Fixes #4447.
Part of #4271.
---
The primary reason of this commit is to overcome a startup hang on
CentOS 6.
https://github.com/tarantool/tarantool/issues/4428
https://github.com/tarantool/tarantool/issues/4447
https://github.com/tarantool/tarantool/issues/4271
https://github.com/tarantool/tarantool/tree/Totktonada/gh-4447-fix-centos-6-startup-fail-full-ci
Waiting for CI:
https://gitlab.com/tarantool/tarantool/pipelines/77906160
I manually verified that tarantool starts on CentOS 6 after the change.
Also verified that the module can be used now on FreeBSD 12.0.
I understood that the commit formally changes a behaviour that can be
visible for a user: it allows to pass cdata<struct passwd> to
pwd.getgr() and to pass cdata<struct group> to pwd.getgr(). It is
unlikely that one will want to use it or will find this ability by an
occasion. Maybe only with pwd.getpw(1LL) or so, but luajit will report
an error in the case. My intention was to keep the commit short and
avoid refactoring of the module code.
It would be good to refactor it in a future: split cdata<struct passwd>
/ cdata<struct group> acquiring from transformation of them into tables.
I tried to do so, but it moves much code around the module, so I skipped
it. Hope it should not be done in the scope of the issue with CentOS 6.
src/lua/pwd.lua | 15 +++++----------
test/app-tap/pwd.skipcond | 8 --------
2 files changed, 5 insertions(+), 18 deletions(-)
delete mode 100644 test/app-tap/pwd.skipcond
diff --git a/src/lua/pwd.lua b/src/lua/pwd.lua
index b6de1562f..ab0229a72 100644
--- a/src/lua/pwd.lua
+++ b/src/lua/pwd.lua
@@ -112,7 +112,7 @@ local function getgr(gid)
if gid == nil then
gid = tonumber(ffi.C.getgid())
end
- local gr = _getgr(gid)
+ local gr = type(gid) == 'cdata' and gid or _getgr(gid)
if gr == nil then
if errno() ~= 0 then
error(pwgr_errstr:format('gr', errno(), errno.strerror()))
@@ -141,7 +141,7 @@ local function getpw(uid)
if uid == nil then
uid = tonumber(ffi.C.getuid())
end
- local pw = _getpw(uid)
+ local pw = type(uid) == 'cdata' and uid or _getpw(uid)
if pw == nil then
if errno() ~= 0 then
error(pwgr_errstr:format('pw', errno(), errno.strerror()))
@@ -170,7 +170,7 @@ local function getpwall()
end
break
end
- table.insert(pws, getpw(pw.pw_uid))
+ table.insert(pws, getpw(pw))
end
ffi.C.endpwent()
return pws
@@ -188,7 +188,7 @@ local function getgrall()
end
break
end
- table.insert(grs, getpw(gr.gr_gid))
+ table.insert(grs, getgr(gr))
end
ffi.C.endgrent()
return grs
@@ -200,12 +200,7 @@ end
-- password database is traversed first time.
--
-- [1]: https://github.com/systemd/systemd/issues/9585
---
--- It is disabled on FreeBSD due to gh-4428: getpwall() hangs on
--- FreeBSD 12.
-if jit.os ~= 'BSD' then
- pcall(getpwall)
-end
+pcall(getpwall)
return {
getpw = getpw,
diff --git a/test/app-tap/pwd.skipcond b/test/app-tap/pwd.skipcond
deleted file mode 100644
index 7950a5d93..000000000
--- a/test/app-tap/pwd.skipcond
+++ /dev/null
@@ -1,8 +0,0 @@
-import platform
-
-# Disabled on FreeBSD due to fail #4271:
-# Data segment size exceeds process limit
-if platform.system() == 'FreeBSD':
- self.skip = 1
-
-# vim: set ft=python:
--
2.22.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH] lua: pwd: fix passwd and group traversal
2019-08-23 1:57 [tarantool-patches] [PATCH] lua: pwd: fix passwd and group traversal Alexander Turenko
@ 2019-08-23 20:26 ` Vladislav Shpilevoy
2019-08-25 11:07 ` Alexander Turenko
2019-08-26 15:08 ` Kirill Yukhin
1 sibling, 1 reply; 6+ messages in thread
From: Vladislav Shpilevoy @ 2019-08-23 20:26 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
Hi! Thanks for the patch!
> The primary reason of this commit is to overcome a startup hang on
> CentOS 6.
>
> https://github.com/tarantool/tarantool/issues/4428
> https://github.com/tarantool/tarantool/issues/4447
> https://github.com/tarantool/tarantool/issues/4271
> https://github.com/tarantool/tarantool/tree/Totktonada/gh-4447-fix-centos-6-startup-fail-full-ci
>
> Waiting for CI:
> https://gitlab.com/tarantool/tarantool/pipelines/77906160
>
> I manually verified that tarantool starts on CentOS 6 after the change.
> Also verified that the module can be used now on FreeBSD 12.0.
>
> I understood that the commit formally changes a behaviour that can be
> visible for a user: it allows to pass cdata<struct passwd> to
> pwd.getgr() and to pass cdata<struct group> to pwd.getgr(). It is
> unlikely that one will want to use it or will find this ability by an
> occasion. Maybe only with pwd.getpw(1LL) or so, but luajit will report
> an error in the case. My intention was to keep the commit short and
> avoid refactoring of the module code.
But this module is not a rocket science, even its whole rewrite would
not be hard. Here the refactoring is quite simple, please, consider
my proposal on the branch and below. You can squash it or keep - anyway
LGTM. Note, I didn't test my 'fixes', so perhaps they are totally wrong.
=========================================================================
diff --git a/src/lua/pwd.lua b/src/lua/pwd.lua
index ab0229a72..7a585a3f2 100644
--- a/src/lua/pwd.lua
+++ b/src/lua/pwd.lua
@@ -108,17 +108,7 @@ end
local pwgr_errstr = "get%s failed [errno %d]: %s"
-local function getgr(gid)
- if gid == nil then
- gid = tonumber(ffi.C.getgid())
- end
- local gr = type(gid) == 'cdata' and gid or _getgr(gid)
- if gr == nil then
- if errno() ~= 0 then
- error(pwgr_errstr:format('gr', errno(), errno.strerror()))
- end
- return nil
- end
+local function grtotable(gr)
local gr_mem, group_members = gr.gr_mem, {}
local i = 0
while true do
@@ -137,25 +127,42 @@ local function getgr(gid)
return group
end
-local function getpw(uid)
- if uid == nil then
- uid = tonumber(ffi.C.getuid())
+local function getgr(gid)
+ if gid == nil then
+ gid = tonumber(ffi.C.getgid())
end
- local pw = type(uid) == 'cdata' and uid or _getpw(uid)
- if pw == nil then
+ local gr = _getgr(gid)
+ if gr == nil then
if errno() ~= 0 then
- error(pwgr_errstr:format('pw', errno(), errno.strerror()))
+ error(pwgr_errstr:format('gr', errno(), errno.strerror()))
end
return nil
end
- local user = {
+ return grtotable(gr)
+end
+
+local function pwtotable(pw)
+ return {
name = ffi.string(pw.pw_name),
id = tonumber(pw.pw_uid),
group = getgr(pw.pw_gid),
workdir = ffi.string(pw.pw_dir),
shell = ffi.string(pw.pw_shell),
}
- return user
+end
+
+local function getpw(uid)
+ if uid == nil then
+ uid = tonumber(ffi.C.getuid())
+ end
+ local pw = _getpw(uid)
+ if pw == nil then
+ if errno() ~= 0 then
+ error(pwgr_errstr:format('pw', errno(), errno.strerror()))
+ end
+ return nil
+ end
+ return pwtotable(pw)
end
local function getpwall()
@@ -170,7 +177,7 @@ local function getpwall()
end
break
end
- table.insert(pws, getpw(pw))
+ table.insert(pws, pwtotable(pw))
end
ffi.C.endpwent()
return pws
@@ -188,7 +195,7 @@ local function getgrall()
end
break
end
- table.insert(grs, getgr(gr))
+ table.insert(grs, grtotable(gr))
end
ffi.C.endgrent()
return grs
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH] lua: pwd: fix passwd and group traversal
2019-08-23 20:26 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2019-08-25 11:07 ` Alexander Turenko
2019-08-25 18:03 ` Vladislav Shpilevoy
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Turenko @ 2019-08-25 11:07 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches, Kirill Yukhin
On Fri, Aug 23, 2019 at 10:26:05PM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
>
> > The primary reason of this commit is to overcome a startup hang on
> > CentOS 6.
> >
> > https://github.com/tarantool/tarantool/issues/4428
> > https://github.com/tarantool/tarantool/issues/4447
> > https://github.com/tarantool/tarantool/issues/4271
> > https://github.com/tarantool/tarantool/tree/Totktonada/gh-4447-fix-centos-6-startup-fail-full-ci
> >
> > Waiting for CI:
> > https://gitlab.com/tarantool/tarantool/pipelines/77906160
> >
> > I manually verified that tarantool starts on CentOS 6 after the change.
> > Also verified that the module can be used now on FreeBSD 12.0.
> >
> > I understood that the commit formally changes a behaviour that can be
> > visible for a user: it allows to pass cdata<struct passwd> to
> > pwd.getgr() and to pass cdata<struct group> to pwd.getgr(). It is
> > unlikely that one will want to use it or will find this ability by an
> > occasion. Maybe only with pwd.getpw(1LL) or so, but luajit will report
> > an error in the case. My intention was to keep the commit short and
> > avoid refactoring of the module code.
>
> But this module is not a rocket science, even its whole rewrite would
> not be hard. Here the refactoring is quite simple, please, consider
> my proposal on the branch and below. You can squash it or keep - anyway
> LGTM. Note, I didn't test my 'fixes', so perhaps they are totally wrong.
>
> =========================================================================
> +local function pwtotable(pw)
> + return {
> name = ffi.string(pw.pw_name),
> id = tonumber(pw.pw_uid),
> group = getgr(pw.pw_gid),
I did the exactly same, but then observed that getgr() call should not
be part of pwtotable() function.
Okay, I understood that you feel uncomfortable with the change of public
API. I'll prepare a patch for refactoring, but I'll ask Kirill to push
the small fix on Monday: two clients already ask us about the CentOS 6
problem.
Hope you don't mind.
Kirill, please consider the discussion and push the small fix.
I'll come back with a refactoring patch soon (hopefully even today).
WBR, Alexander Turenko.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH] lua: pwd: fix passwd and group traversal
2019-08-25 11:07 ` Alexander Turenko
@ 2019-08-25 18:03 ` Vladislav Shpilevoy
2019-08-26 9:43 ` Alexander Turenko
0 siblings, 1 reply; 6+ messages in thread
From: Vladislav Shpilevoy @ 2019-08-25 18:03 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches, Kirill Yukhin
On 25/08/2019 13:07, Alexander Turenko wrote:
> On Fri, Aug 23, 2019 at 10:26:05PM +0200, Vladislav Shpilevoy wrote:
>> Hi! Thanks for the patch!
>>
>>> The primary reason of this commit is to overcome a startup hang on
>>> CentOS 6.
>>>
>>> https://github.com/tarantool/tarantool/issues/4428
>>> https://github.com/tarantool/tarantool/issues/4447
>>> https://github.com/tarantool/tarantool/issues/4271
>>> https://github.com/tarantool/tarantool/tree/Totktonada/gh-4447-fix-centos-6-startup-fail-full-ci
>>>
>>> Waiting for CI:
>>> https://gitlab.com/tarantool/tarantool/pipelines/77906160
>>>
>>> I manually verified that tarantool starts on CentOS 6 after the change.
>>> Also verified that the module can be used now on FreeBSD 12.0.
>>>
>>> I understood that the commit formally changes a behaviour that can be
>>> visible for a user: it allows to pass cdata<struct passwd> to
>>> pwd.getgr() and to pass cdata<struct group> to pwd.getgr(). It is
>>> unlikely that one will want to use it or will find this ability by an
>>> occasion. Maybe only with pwd.getpw(1LL) or so, but luajit will report
>>> an error in the case. My intention was to keep the commit short and
>>> avoid refactoring of the module code.
>>
>> But this module is not a rocket science, even its whole rewrite would
>> not be hard. Here the refactoring is quite simple, please, consider
>> my proposal on the branch and below. You can squash it or keep - anyway
>> LGTM. Note, I didn't test my 'fixes', so perhaps they are totally wrong.
>>
>> =========================================================================
>
>> +local function pwtotable(pw)
>> + return {
>> name = ffi.string(pw.pw_name),
>> id = tonumber(pw.pw_uid),
>> group = getgr(pw.pw_gid),
>
> I did the exactly same, but then observed that getgr() call should not
> be part of pwtotable() function.
Why? It does not affect pw iterators.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH] lua: pwd: fix passwd and group traversal
2019-08-25 18:03 ` Vladislav Shpilevoy
@ 2019-08-26 9:43 ` Alexander Turenko
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Turenko @ 2019-08-26 9:43 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches, Kirill Yukhin
On Sun, Aug 25, 2019 at 08:03:13PM +0200, Vladislav Shpilevoy wrote:
>
>
> On 25/08/2019 13:07, Alexander Turenko wrote:
> > On Fri, Aug 23, 2019 at 10:26:05PM +0200, Vladislav Shpilevoy wrote:
> >> Hi! Thanks for the patch!
> >>
> >>> The primary reason of this commit is to overcome a startup hang on
> >>> CentOS 6.
> >>>
> >>> https://github.com/tarantool/tarantool/issues/4428
> >>> https://github.com/tarantool/tarantool/issues/4447
> >>> https://github.com/tarantool/tarantool/issues/4271
> >>> https://github.com/tarantool/tarantool/tree/Totktonada/gh-4447-fix-centos-6-startup-fail-full-ci
> >>>
> >>> Waiting for CI:
> >>> https://gitlab.com/tarantool/tarantool/pipelines/77906160
> >>>
> >>> I manually verified that tarantool starts on CentOS 6 after the change.
> >>> Also verified that the module can be used now on FreeBSD 12.0.
> >>>
> >>> I understood that the commit formally changes a behaviour that can be
> >>> visible for a user: it allows to pass cdata<struct passwd> to
> >>> pwd.getgr() and to pass cdata<struct group> to pwd.getgr(). It is
> >>> unlikely that one will want to use it or will find this ability by an
> >>> occasion. Maybe only with pwd.getpw(1LL) or so, but luajit will report
> >>> an error in the case. My intention was to keep the commit short and
> >>> avoid refactoring of the module code.
> >>
> >> But this module is not a rocket science, even its whole rewrite would
> >> not be hard. Here the refactoring is quite simple, please, consider
> >> my proposal on the branch and below. You can squash it or keep - anyway
> >> LGTM. Note, I didn't test my 'fixes', so perhaps they are totally wrong.
> >>
> >> =========================================================================
> >
> >> +local function pwtotable(pw)
> >> + return {
> >> name = ffi.string(pw.pw_name),
> >> id = tonumber(pw.pw_uid),
> >> group = getgr(pw.pw_gid),
> >
> > I did the exactly same, but then observed that getgr() call should not
> > be part of pwtotable() function.
>
> Why? It does not affect pw iterators.
It'll affect group iterator.
My idea was to split receiveing of data from 'deserialization'.
WBR, Alexander Turenko.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH] lua: pwd: fix passwd and group traversal
2019-08-23 1:57 [tarantool-patches] [PATCH] lua: pwd: fix passwd and group traversal Alexander Turenko
2019-08-23 20:26 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2019-08-26 15:08 ` Kirill Yukhin
1 sibling, 0 replies; 6+ messages in thread
From: Kirill Yukhin @ 2019-08-26 15:08 UTC (permalink / raw)
To: tarantool-patches; +Cc: Vladislav Shpilevoy, Alexander Turenko
Hello,
On 23 авг 04:57, Alexander Turenko wrote:
> CentOS 6 and FreeBSD 12 implementations of getpwuid() rewind
> setpwent()-{getpwent()}-endpwent() loop to a start that leads to a hang
> during pwd.getpwall() invoke. The same is true for a getgrgid() call
> during setgrent()-{getgrent()}-endgrent() loop.
>
> The commit modifies pwd module to avoid getpwuid() calls during passwd
> database traversal and to avoid getgrgid() calls when traversing groups.
>
> The commit also fixes the important regression on CentOS 6 after
> f5d8331e833d6a29dc7a869e681e03fb8e03a5f8 ('lua: workaround
> pwd.getpwall() issue on Fedora 29'): tarantool hungs during startup due
> to added getpwall() call. This made tarantool unusable on CentOS 6 at
> all.
>
> Aside of that the commit fixes another pwd.getgrall() problem: the
> function gaves password entries instead of group entries.
>
> Fixes #4428.
> Fixes #4447.
> Part of #4271.
I've checked the patch into 1.10, 2.1, 2.2 and master.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-26 15:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-23 1:57 [tarantool-patches] [PATCH] lua: pwd: fix passwd and group traversal Alexander Turenko
2019-08-23 20:26 ` [tarantool-patches] " Vladislav Shpilevoy
2019-08-25 11:07 ` Alexander Turenko
2019-08-25 18:03 ` Vladislav Shpilevoy
2019-08-26 9:43 ` Alexander Turenko
2019-08-26 15:08 ` Kirill Yukhin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox