On Wed, Aug 3, 2016 at 5:10 PM, Jakub Hrozek <[email protected]> wrote:
> On Tue, Aug 02, 2016 at 04:08:03PM +0200, Fabiano Fidêncio wrote:
>> So, sorry for the very long title and please, see the proposed patches
>> for solving https://fedorahosted.org/sssd/ticket/3094
>>
>>
>> Best Regards,
>> --
>> Fabiano Fidêncio
>
>> From 7b04832fce0bdf7f8e86c30f60811f7d219e62fc Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]>
>> Date: Tue, 2 Aug 2016 15:04:51 +0200
>> Subject: [PATCH 1/3] RESPONDERS: Decrease debug level for failures in
>
> ACK
>
>> From ac7d4a7e6dfaf5d6c08419452b36937ac01453be Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]>
>> Date: Tue, 2 Aug 2016 15:08:09 +0200
>> Subject: [PATCH 2/3] RESPONDERS: Show a bit more info in case of
>
> ACK
>
>> From 8d595daa03b2a8af1605744cd19d5000a9546456 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]>
>> Date: Tue, 2 Aug 2016 15:12:45 +0200
>> Subject: [PATCH 3/3] RESPONDERS: Pass errno to strerror() when
>> SELINUX_getpeercon() fails
>
> I would prefer:
> ret = errno;
> strerror(ret)
>
> in case we ever added another IO operation into the block which might
> reset errno.
Okay. Suggestion taken.
I've attached a v2 of all patches.
There are no changes in the already acked patches.
For the 3rd patch that you suggested a change, here is the diff
between the v2 and v1, just in case it makes your review easier:
[ffidenci@cat sssd]$ git diff
diff --git a/src/responder/common/responder_common.c
b/src/responder/common/responder_common.c
index a3ad9ad..05e6cee 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -128,10 +128,11 @@ static errno_t get_client_cred(struct cli_ctx *cctx)
ret = SELINUX_getpeercon(cctx->cfd, &secctx);
if (ret != 0) {
+ ret = errno;
DEBUG(SSSDBG_MINOR_FAILURE,
"The following failure is expected to happen in case
SELinux is disabled:\n"
"SELINUX_getpeercon failed [%d][%s].\n"
- "Please, consider enabling SELinux in your system.\n",
ret, strerror(errno));
+ "Please, consider enabling SELinux in your system.\n",
ret, strerror(ret));
/* This is not fatal, as SELinux may simply be disabled */
ret = EOK;
} else {
Best Regards,
--
Fabiano Fidêncio
From 4641d8f841da0147a16df9f19c19a8732126d40d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]>
Date: Tue, 2 Aug 2016 15:04:51 +0200
Subject: [PATCH v2 1/3] RESPONDERS: Decrease debug level for failures in
SELINUX_getpeercon()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
As this is not FATAL and may happen when SELinux is disabled, let's just
decrease the debug level to MINOR_FAILURE
Signed-off-by: Fabiano Fidêncio <[email protected]>
Related:
https://fedorahosted.org/sssd/ticket/3094
---
src/responder/common/responder_common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index 7f6264a..e7590bc 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -128,8 +128,8 @@ static errno_t get_client_cred(struct cli_ctx *cctx)
ret = SELINUX_getpeercon(cctx->cfd, &secctx);
if (ret != 0) {
- DEBUG(SSSDBG_CRIT_FAILURE,
- "SELINUX_getpeercon failed [%d][%s].\n", ret, strerror(ret));
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "SELINUX_getpeercon failed [%d][%s].\n"
/* This is not fatal, as SELinux may simply be disabled */
ret = EOK;
} else {
--
2.7.4
From 480e57dbce6ccda093d68802bed15b530907ecf9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]>
Date: Tue, 2 Aug 2016 15:08:09 +0200
Subject: [PATCH v2 2/3] RESPONDERS: Show a bit more info in case of
SELINUX_getpeercon() failure
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Be explicit that it may happen when SELinux is disabled and also suggest
to enable SELinux.
Signed-off-by: Fabiano Fidêncio <[email protected]>
Related:
https://fedorahosted.org/sssd/ticket/3094
---
src/responder/common/responder_common.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index e7590bc..8d9499d 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -129,7 +129,9 @@ static errno_t get_client_cred(struct cli_ctx *cctx)
ret = SELINUX_getpeercon(cctx->cfd, &secctx);
if (ret != 0) {
DEBUG(SSSDBG_MINOR_FAILURE,
+ "The following failure is expected to happen in case SELinux is disabled:\n"
"SELINUX_getpeercon failed [%d][%s].\n"
+ "Please, consider enabling SELinux in your system.\n", ret, strerror(ret));
/* This is not fatal, as SELinux may simply be disabled */
ret = EOK;
} else {
--
2.7.4
From 7bb6e6b4d0908674eac01e2a1806b64d57f6053f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]>
Date: Tue, 2 Aug 2016 15:12:45 +0200
Subject: [PATCH v2 3/3] RESPONDERS: Pass errno to strerror() when
SELINUX_getpeercon() fails
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Currently ret, which is -1, is passed to strerror() instead of errno.
Signed-off-by: Fabiano Fidêncio <[email protected]>
Related:
https://fedorahosted.org/sssd/ticket/3094
---
src/responder/common/responder_common.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index 8d9499d..05e6cee 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -128,6 +128,7 @@ static errno_t get_client_cred(struct cli_ctx *cctx)
ret = SELINUX_getpeercon(cctx->cfd, &secctx);
if (ret != 0) {
+ ret = errno;
DEBUG(SSSDBG_MINOR_FAILURE,
"The following failure is expected to happen in case SELinux is disabled:\n"
"SELINUX_getpeercon failed [%d][%s].\n"
--
2.7.4
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]
