Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
Hi
On Tue, Nov 11, 2014 at 11:33 AM, Susant Sahani sus...@redhat.com wrote:
Unchecked return value from library
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..c4cd387 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,7 +376,9 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+r = mkfifo(p, 0600);
+if (r 0)
+return -errno;
What if that fifo already exists? Like if tty-ask-password-agent
crashes and is restarted? Maybe fix both calls, mkdir_parents_label()
and mkfifo(), to ignore the return value via (void).
Or am I missing something?
Thanks
David
fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
if (fd 0)
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
Hi
On Mon, Nov 17, 2014 at 11:20 AM, Susant Sahani sus...@redhat.com wrote:
On 11/17/2014 03:39 PM, David Herrmann wrote:
Hi
Hi David,
On Tue, Nov 11, 2014 at 11:33 AM, Susant Sahani sus...@redhat.com wrote:
Unchecked return value from library
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..c4cd387 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,7 +376,9 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+r = mkfifo(p, 0600);
+if (r 0)
+return -errno;
What if that fifo already exists? Like if tty-ask-password-agent
crashes and is restarted? Maybe fix both calls, mkdir_parents_label()
and mkfifo(), to ignore the return value via (void).
yes I forgot that Thanks . In this case I guess
r = mkfifo(p, 0600);
if (r 0) {
if(errno != EEXIST)
return -errno;
}
would be better.
Maybe just use if (r 0 errno != EEXIST)
Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..1fc792b 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+(void)mkfifo(p, 0600);
fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
if (fd 0)
return -errno;
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..1fc792b 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+(void)mkfifo(p, 0600);
You really aren't fixing anything in these patches, just merely
papering over the Coverity issues. Which is fine, if you really want to
do that, but don't think it's anything other than that...
thanks,
greg k-h
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
On 11/17/2014 10:26 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..1fc792b 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+(void)mkfifo(p, 0600);
You really aren't fixing anything in these patches, just merely
papering over the Coverity issues. Which is fine, if you really want to
do that, but don't think it's anything other than that...
Yes my intention is to for coverity only Any way next line 'open'
handling the error case .
Susant
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
On Mon, Nov 17, 2014 at 10:36:53PM +0530, Susant Sahani wrote:
On 11/17/2014 10:26 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..1fc792b 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+(void)mkfifo(p, 0600);
You really aren't fixing anything in these patches, just merely
papering over the Coverity issues. Which is fine, if you really want to
do that, but don't think it's anything other than that...
Yes my intention is to for coverity only Any way next line 'open' handling
the error case .
I'm sorry, but I don't understand this sentance at all, can you rephrase
it?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
On 11/17/2014 10:39 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 10:36:53PM +0530, Susant Sahani wrote:
On 11/17/2014 10:26 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..1fc792b 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+(void)mkfifo(p, 0600);
You really aren't fixing anything in these patches, just merely
papering over the Coverity issues. Which is fine, if you really want to
do that, but don't think it's anything other than that...
Yes my intention is to for coverity only Any way next line 'open' handling
the error case .
I'm sorry, but I don't understand this sentance at all, can you rephrase
it?
Sorry let me rephrase it. This patch only for coverity . The next like
of mkfifo is open .
(void)mkfifo(p, 0600);
fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
if (fd 0)
return -errno;
and open is handling the failure.
Susant
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
On 11/17/2014 10:39 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 10:36:53PM +0530, Susant Sahani wrote:
On 11/17/2014 10:26 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..1fc792b 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+(void)mkfifo(p, 0600);
You really aren't fixing anything in these patches, just merely
papering over the Coverity issues. Which is fine, if you really want to
do that, but don't think it's anything other than that...
Yes my intention is to for coverity only Any way next line 'open' handling
the error case .
I'm sorry, but I don't understand this sentance at all, can you rephrase
it?
Sorry let me rephrase it. This patch only for coverity . The next line
of code mkfifo is open .
(void)mkfifo(p, 0600);
fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
if (fd 0)
return -errno;
and open is handling the failure.
Susant
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
On Mon, Nov 17, 2014 at 10:44:14PM +0530, Susant Sahani wrote:
On 11/17/2014 10:39 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 10:36:53PM +0530, Susant Sahani wrote:
On 11/17/2014 10:26 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..1fc792b 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+(void)mkfifo(p, 0600);
You really aren't fixing anything in these patches, just merely
papering over the Coverity issues. Which is fine, if you really want to
do that, but don't think it's anything other than that...
Yes my intention is to for coverity only Any way next line 'open' handling
the error case .
I'm sorry, but I don't understand this sentance at all, can you rephrase
it?
Sorry let me rephrase it. This patch only for coverity . The next like of
mkfifo is open .
(void)mkfifo(p, 0600);
fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
if (fd 0)
return -errno;
and open is handling the failure.
Then coverity should be fixed, don't paper over stupid bugs in tools for
no reason.
thanks,
greg k-h
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
2014-11-17 18:31 GMT+01:00 Greg KH gre...@linuxfoundation.org:
On Mon, Nov 17, 2014 at 10:44:14PM +0530, Susant Sahani wrote:
On 11/17/2014 10:39 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 10:36:53PM +0530, Susant Sahani wrote:
On 11/17/2014 10:26 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..1fc792b 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+(void)mkfifo(p, 0600);
You really aren't fixing anything in these patches, just merely
papering over the Coverity issues. Which is fine, if you really want to
do that, but don't think it's anything other than that...
Yes my intention is to for coverity only Any way next line 'open' handling
the error case .
I'm sorry, but I don't understand this sentance at all, can you rephrase
it?
Sorry let me rephrase it. This patch only for coverity . The next like of
mkfifo is open .
(void)mkfifo(p, 0600);
fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
if (fd 0)
return -errno;
and open is handling the failure.
Then coverity should be fixed, don't paper over stupid bugs in tools for
no reason.
I disagree.
Coverity can not infer this in any possible way. How can coverity
infer that we do not care about the return value of mkfifo ?
It really depends of the semantic here. In this case Susant is
documenting the fact that he does not care about the return value of
mkfifo because he thinks that it is already handled by open. In
another program one can just forgot to check the return value of
mkfifo and doing an open after, but maybe in this program checking the
return value of mkfifo is important.
thanks,
greg k-h
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
On Mon, Nov 17, 2014 at 06:47:33PM +0100, Ronny Chevalier wrote:
2014-11-17 18:31 GMT+01:00 Greg KH gre...@linuxfoundation.org:
On Mon, Nov 17, 2014 at 10:44:14PM +0530, Susant Sahani wrote:
On 11/17/2014 10:39 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 10:36:53PM +0530, Susant Sahani wrote:
On 11/17/2014 10:26 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..1fc792b 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+(void)mkfifo(p, 0600);
You really aren't fixing anything in these patches, just merely
papering over the Coverity issues. Which is fine, if you really want to
do that, but don't think it's anything other than that...
Yes my intention is to for coverity only Any way next line 'open'
handling
the error case .
I'm sorry, but I don't understand this sentance at all, can you rephrase
it?
Sorry let me rephrase it. This patch only for coverity . The next like of
mkfifo is open .
(void)mkfifo(p, 0600);
fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
if (fd 0)
return -errno;
and open is handling the failure.
Then coverity should be fixed, don't paper over stupid bugs in tools for
no reason.
I disagree.
Coverity can not infer this in any possible way. How can coverity
infer that we do not care about the return value of mkfifo ?
It really depends of the semantic here.
Coverity is a semantic checker, why can't it be changed to determine
if mkfifo() is followed by open() and an error check, that it is safe
code? It does this for lots of other common patterns.
greg k-h
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
On 11/18/2014 12:06 AM, Greg KH wrote:
On Mon, Nov 17, 2014 at 06:47:33PM +0100, Ronny Chevalier wrote:
2014-11-17 18:31 GMT+01:00 Greg KH gre...@linuxfoundation.org:
On Mon, Nov 17, 2014 at 10:44:14PM +0530, Susant Sahani wrote:
On 11/17/2014 10:39 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 10:36:53PM +0530, Susant Sahani wrote:
On 11/17/2014 10:26 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..1fc792b 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+(void)mkfifo(p, 0600);
You really aren't fixing anything in these patches, just merely
papering over the Coverity issues. Which is fine, if you really want to
do that, but don't think it's anything other than that...
Yes my intention is to for coverity only Any way next line 'open' handling
the error case .
I'm sorry, but I don't understand this sentance at all, can you rephrase
it?
Sorry let me rephrase it. This patch only for coverity . The next like of
mkfifo is open .
(void)mkfifo(p, 0600);
fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
if (fd 0)
return -errno;
and open is handling the failure.
Then coverity should be fixed, don't paper over stupid bugs in tools for
no reason.
I disagree.
Coverity can not infer this in any possible way. How can coverity
infer that we do not care about the return value of mkfifo ?
It really depends of the semantic here.
Coverity is a semantic checker, why can't it be changed to determine
if mkfifo() is followed by open() and an error check, that it is safe
code? It does this for lots of other common patterns.
For now mkfifo/mkdir/ioctl coverity is not that smart or is it ? From
the behaviour of coverity It looks for single statement in these
scenario . The mkfifo could be one function then this fifo can be used
some other function like open or read/write. There are several scenario
would be like this .
Susant
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
2014-11-17 19:36 GMT+01:00 Greg KH gre...@linuxfoundation.org:
On Mon, Nov 17, 2014 at 06:47:33PM +0100, Ronny Chevalier wrote:
2014-11-17 18:31 GMT+01:00 Greg KH gre...@linuxfoundation.org:
On Mon, Nov 17, 2014 at 10:44:14PM +0530, Susant Sahani wrote:
On 11/17/2014 10:39 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 10:36:53PM +0530, Susant Sahani wrote:
On 11/17/2014 10:26 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..1fc792b 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+(void)mkfifo(p, 0600);
You really aren't fixing anything in these patches, just merely
papering over the Coverity issues. Which is fine, if you really want
to
do that, but don't think it's anything other than that...
Yes my intention is to for coverity only Any way next line 'open'
handling
the error case .
I'm sorry, but I don't understand this sentance at all, can you rephrase
it?
Sorry let me rephrase it. This patch only for coverity . The next like of
mkfifo is open .
(void)mkfifo(p, 0600);
fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
if (fd 0)
return -errno;
and open is handling the failure.
Then coverity should be fixed, don't paper over stupid bugs in tools for
no reason.
I disagree.
Coverity can not infer this in any possible way. How can coverity
infer that we do not care about the return value of mkfifo ?
It really depends of the semantic here.
Coverity is a semantic checker, why can't it be changed to determine
if mkfifo() is followed by open() and an error check, that it is safe
code? It does this for lots of other common patterns.
For me I see this as a warning, for some cases it is safe and there is
no problem like this one so we can document the code for us and tools
like Coverity, but it can be a mistake and maybe it should have been
checked. So Coverity assumes the worst case by warning us, and I don't
see the problem.
greg k-h
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
On Tue, Nov 18, 2014 at 12:21:29AM +0530, Susant Sahani wrote:
On 11/18/2014 12:06 AM, Greg KH wrote:
On Mon, Nov 17, 2014 at 06:47:33PM +0100, Ronny Chevalier wrote:
2014-11-17 18:31 GMT+01:00 Greg KH gre...@linuxfoundation.org:
On Mon, Nov 17, 2014 at 10:44:14PM +0530, Susant Sahani wrote:
On 11/17/2014 10:39 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 10:36:53PM +0530, Susant Sahani wrote:
On 11/17/2014 10:26 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..1fc792b 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+(void)mkfifo(p, 0600);
You really aren't fixing anything in these patches, just merely
papering over the Coverity issues. Which is fine, if you really want
to
do that, but don't think it's anything other than that...
Yes my intention is to for coverity only Any way next line 'open'
handling
the error case .
I'm sorry, but I don't understand this sentance at all, can you rephrase
it?
Sorry let me rephrase it. This patch only for coverity . The next like of
mkfifo is open .
(void)mkfifo(p, 0600);
fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
if (fd 0)
return -errno;
and open is handling the failure.
Then coverity should be fixed, don't paper over stupid bugs in tools for
no reason.
I disagree.
Coverity can not infer this in any possible way. How can coverity
infer that we do not care about the return value of mkfifo ?
It really depends of the semantic here.
Coverity is a semantic checker, why can't it be changed to determine
if mkfifo() is followed by open() and an error check, that it is safe
code? It does this for lots of other common patterns.
For now mkfifo/mkdir/ioctl coverity is not that smart or is it ?
Talk to the coverity people. Given that it is a closed source tool,
that costs money, I am very loath to do anything to make it better,
and I really don't like it forcing programs to work around its
deficiencies.
greg k-h
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
On Mon, Nov 17, 2014 at 7:54 PM, Greg KH gre...@linuxfoundation.org wrote:
On Tue, Nov 18, 2014 at 12:21:29AM +0530, Susant Sahani wrote:
On 11/18/2014 12:06 AM, Greg KH wrote:
On Mon, Nov 17, 2014 at 06:47:33PM +0100, Ronny Chevalier wrote:
2014-11-17 18:31 GMT+01:00 Greg KH gre...@linuxfoundation.org:
On Mon, Nov 17, 2014 at 10:44:14PM +0530, Susant Sahani wrote:
On 11/17/2014 10:39 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 10:36:53PM +0530, Susant Sahani wrote:
On 11/17/2014 10:26 PM, Greg KH wrote:
On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..1fc792b 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+(void)mkfifo(p, 0600);
You really aren't fixing anything in these patches, just merely
papering over the Coverity issues. Which is fine, if you really want
to
do that, but don't think it's anything other than that...
Yes my intention is to for coverity only Any way next line 'open'
handling
the error case .
I'm sorry, but I don't understand this sentance at all, can you rephrase
it?
Sorry let me rephrase it. This patch only for coverity . The next like of
mkfifo is open .
(void)mkfifo(p, 0600);
fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
if (fd 0)
return -errno;
and open is handling the failure.
Then coverity should be fixed, don't paper over stupid bugs in tools for
no reason.
I disagree.
Coverity can not infer this in any possible way. How can coverity
infer that we do not care about the return value of mkfifo ?
It really depends of the semantic here.
Coverity is a semantic checker, why can't it be changed to determine
if mkfifo() is followed by open() and an error check, that it is safe
code? It does this for lots of other common patterns.
For now mkfifo/mkdir/ioctl coverity is not that smart or is it ?
Talk to the coverity people. Given that it is a closed source tool,
that costs money, I am very loath to do anything to make it better,
and I really don't like it forcing programs to work around its
deficiencies.
greg k-h
What coverity is complaining about in this CID is this:
Unchecked return value from library. Calling mkfifo() without
checking return value. This library function may fail and return an
error code.
We can choose to either make it explicit that we don't care about the
return value with this patch, or we can just mark the issue as
intentional in coverity to make it go away. The (void) is a bit ugly
imo but it does show that ignoring the return was a conscious
decision. A matter of taste I guess.
- Thomas
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
Unchecked return value from library
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..c4cd387 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,7 +376,9 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+r = mkfifo(p, 0600);
+if (r 0)
+return -errno;
fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
if (fd 0)
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
