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

[Greg KH]

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 <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.
> 
> 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