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

[Susant Sahani]

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