search

LLNL / umap

User-space Page Management
GNU Lesser General Public License v2.1
104 stars 22 forks source link

Fix bit-check for UMAP_RO_MODE #98

Closed mrks closed 4 years ago

mrks commented 4 years ago

I just started looking into this project, which seems to solve a couple of problems that we have been working on. Following the instructions from the documentation, the prot PROT_WRITE is not supported in UMAP_RO_MODE compilation assertion was thrown. I didn't explicitly enable UMAP_RO_MODE, so I have no idea why this didn't happen to anyone before.

In any case, I am almost sure that if( prot | PROT_WRITE) is a bug as the bit-or will always return something non-zero.

IBPeng commented 4 years ago

We had an internal fix for this issue that was not checked in. It has been pushed, please let us know if it is still an issue. ( prot & PROT_WRITE ) in your pull request, however, will cause prot=PROT_READ to throw assertion, because PROT_WRITE includes PROT_READ, i.e., ( PROT_READ & PROT_WRITE ) is true.

mrks commented 4 years ago

Are we talking about the same thing? For me, the & change fixed the issue, and the definition looks like they are not overlapping:

/*
 * Protections are chosen from these bits, or-ed together
 */
#define PROT_NONE   0x00    /* no permissions */
#define PROT_READ   0x01    /* pages can be read */
#define PROT_WRITE  0x02    /* pages can be written */
#define PROT_EXEC   0x04    /* pages can be executed */

Any chance you were thinking about this part from the mmap documentation:

On some hardware architectures (e.g., i386), PROT_WRITE implies PROT_READ.

On the API side, that's just a semantical thing and does not affect the values of PROT_*.

Anyway, your commit fixes the problem just as well, at least if we don't care about PROT_EXEC.