[systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261

[Greg KH]

On Mon, Nov 17, 2014 at 06:47:33PM +0100, Ronny Chevalier wrote:
> 2014-11-17 18:31 GMT+01:00 Greg KH <gregkh at 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