Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 643084 - openrc doesn't check for stat return value and assume success
Summary: openrc doesn't check for stat return value and assume success
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Hosted Projects
Classification: Unclassified
Component: OpenRC (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: OpenRC Team
URL: https://github.com/OpenRC/openrc/blob...
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-01-02 10:25 UTC by nobody
Modified: 2018-01-21 19:17 UTC (History)
0 users

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description nobody 2018-01-02 10:25:06 UTC
Hopefully that's the only line when the mistake was done, other use of stat safely use if (stat(..) == 0...

And if you look at line 384, you can see bad result of that assumption, openrc is comparing st.st_mtime < t where st.st_mtime value is at worst random, or default set by the struct.


Reproducible: Always



Expected Results:  
openrc should check that stat return value indicate success and on failure not checking the value of st.st_mtime.
i suppose as the RC_DEPTREE_CACHE is create just upper, someone assume the file exists and nothing could prevent stat from succeed looking at it.
But still, stat could fail (oom, dying hdd, bad fs...)
Comment 1 William Hubbs gentoo-dev 2018-01-09 17:44:57 UTC
There isn't enough information in this report for me to determine where
the issue is.

Which version of OpenRC are you using?
Also, you mention line 384 without saying which file it is in.

Is the issue still in the master branch at
https://github.com/openrc/openrc?

Thanks,

William
Comment 2 nobody 2018-01-11 02:02:50 UTC
I gave the link in the url section of the bug section ;)

here it is again: https://github.com/OpenRC/openrc/blob/master/src/rc/rc-misc.c#L383
Comment 3 nobody 2018-01-14 17:27:23 UTC
You may also fix that https://github.com/OpenRC/openrc/commit/918d955fd2de1f594b83508f5ddd5271534e3591
The commit change use of chown to lchown, but forget to change chown to lchown in the error message

So as tiny as it is, the error message should reflect it use lchown now and no more chown, because lchown will at least return an error lchown generate that chown doesn't : when the file is a symlink, and it will be clearer to see the error code is the one return from lchown instead of chown.
< 			eerror("%s: chown: %s", applet, strerror(errno)); 
> 			eerror("%s: lchown: %s", applet, strerror(errno));
Comment 4 William Hubbs gentoo-dev 2018-01-16 19:46:49 UTC
The below linked commits take care of these issues.

https://github.com/openrc/openrc/commit/4af5a80b
https://github.com/openrc/openrc/commit/87c98ebb

In the future, please open separate bugs for un-related issues like this.

Thanks much.

William
Comment 5 nobody 2018-01-21 19:17:51 UTC
I have estimate the second fix was so tiny it wasn't worth another bug for that.
I will do things properly.

Thank you for your attention and the fixes Mr. Hubbs