Details

    • 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

      • expand all symlinks with realpath()
      • verify that the true path isn't leading into a datadir
      • open (or, accordingly, delete) the file

      This is race condition prone, one can change the symlink after it was expanded by realpath() but before the file is opened.

      Attachments

        Issue Links

          Activity

            svoj, could you please review the final fix? It's a set of commits, the complete diff is

            git diff bb-5.5-serg^{/DEBUG_SYNC}..bb-5.5-serg^{/my_delete_with_symlink}
            

            and you can look at individual commits with

            gitk bb-5.5-serg^{/DEBUG_SYNC}..bb-5.5-serg^{/my_delete_with_symlink}
            

            The main one is "MDEV-11902 mi_open race condition" and the four topmost ones (about my_delete).

            serg Sergei Golubchik added a comment - svoj , could you please review the final fix? It's a set of commits, the complete diff is git diff bb-5.5-serg^{/DEBUG_SYNC}..bb-5.5-serg^{/my_delete_with_symlink} and you can look at individual commits with gitk bb-5.5-serg^{/DEBUG_SYNC}..bb-5.5-serg^{/my_delete_with_symlink} The main one is " MDEV-11902 mi_open race condition" and the four topmost ones (about my_delete).

            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.

            svoj Sergey Vojtovich added a comment - 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.

            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.

            serg Sergei Golubchik added a comment - 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.

            I was mostly unhappy about function definition hidden behind a macro. Otherwise I'm more or less fine with this big NOSYMLINK_FUNCTION_BODY.

            svoj Sergey Vojtovich added a comment - I was mostly unhappy about function definition hidden behind a macro. Otherwise I'm more or less fine with this big NOSYMLINK_FUNCTION_BODY.

            People

              serg Sergei Golubchik
              serg Sergei Golubchik
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.