[MDEV-11902] mi_open race condition Created: 2017-01-24 Updated: 2017-04-18 Resolved: 2017-02-27 |
|
| Status: | Closed |
| Project: | MariaDB Server |
| Component/s: | Data Definition - Alter Table |
| Affects Version/s: | 5.5, 10.0, 10.1, 10.2 |
| Fix Version/s: | 5.5.55, 10.0.30, 10.1.22, 10.2.5 |
| Type: | Bug | Priority: | Blocker |
| Reporter: | Sergei Golubchik | Assignee: | Sergei Golubchik |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Sprint: | 5.5.55 |
| Description |
|
In one created a MyISAM table with DATA_DIRECTORY or INDEX_DIRECTORY attributes (that is, with symlinks) mi_open() and mi_delete() used the pattern of
This is race condition prone, one can change the symlink after it was expanded by realpath() but before the file is opened. |
| Comments |
| Comment by Sergei Golubchik [ 2017-02-21 ] | ||
|
svoj, could you please review the final fix? It's a set of commits, the complete diff is
and you can look at individual commits with
The main one is " | ||
| Comment by Sergey Vojtovich [ 2017-02-22 ] | ||
|
Looks good. There's a minor typo in "all simlinks are resolved by realpath()". I'm not a big fan of CREATE_NOSYMLINK_FUNCTION. I'd better put NOSYMLINK_FUNCTION_BODY directly into my_delete/etc. Consider it less than a minor wish. Table creation still looks badly vulnerable, FWICS: one may create files in data directory, one may truncate files in data directory, one may delete files in data directory. But we agreed to keep it separately. | ||
| Comment by Sergei Golubchik [ 2017-02-27 ] | ||
|
About CREATE_NOSYMLINK_FUNCTION — yes, originally I had two functions directly in my_open.c and my_delete.c. They were mostly identical, just one line difference. Then I needed to fix a bug there, had to fix it in both places. Then again, and I thought it's rather fragile approach (and I'm generally not a big fan of duplicated code). So I extracted the common part. If there's a better solution — I'll happily use it. | ||
| Comment by Sergey Vojtovich [ 2017-02-27 ] | ||
|
I was mostly unhappy about function definition hidden behind a macro. Otherwise I'm more or less fine with this big NOSYMLINK_FUNCTION_BODY. |