[LU-4969] nodemap key hash comparison is broken Created: 28/Apr/14 Updated: 07/Oct/14 Resolved: 07/Oct/14 |
|
| Status: | Resolved |
| Project: | Lustre |
| Component/s: | None |
| Affects Version/s: | Lustre 2.6.0 |
| Fix Version/s: | Lustre 2.7.0 |
| Type: | Bug | Priority: | Critical |
| Reporter: | Andreas Dilger | Assignee: | Andreas Dilger |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | patch | ||
| Severity: | 3 |
| Rank (Obsolete): | 13758 |
| Description |
|
The key hash comparison function nodemap_hs_keycmp() used for nodemap is broken: static void *nodemap_hs_key(cfs_hlist_node_t *hnode) { struct lu_nodemap *nodemap; nodemap = cfs_hlist_entry(hnode, struct lu_nodemap, nm_hash); return nodemap->nm_name; } static int nodemap_hs_keycmp(const void *key, cfs_hlist_node_t *compared_hnode) { struct lu_nodemap *nodemap; nodemap = nodemap_hs_key(compared_hnode); return !strcmp(key, nodemap->nm_name); } The nodemap_hs_key() function returns a pointer to the nodemap name, but then nodemap_hs_keycmp() dereferences this again to try and get the name. Since this function is only used when resizing the hash table, it would not be hit until the initial hash table size is getting too full (looks like about 128 entries or so). I think this could be fixed by just using the return value from nodemap_hs_key() directly: static int nodemap_hs_keycmp(const void *key, cfs_hlist_node_t *compared_hnode) { return !strcmp(key, nodemap_hs_key(compared_hnode)); } |
| Comments |
| Comment by Andreas Dilger [ 28/Apr/14 ] |
|
Josh, I found this bug while looking at the cfs_hash users in the code. It is a minor fix but should probably be fixed for 2.6 since it would cause the node to crash. That said, I don't think anything but test code will be adding nodemaps yet, so it isn't a 2.6 blocker. |
| Comment by Joshua Walgenbach (Inactive) [ 29/Apr/14 ] |
|
Andreas, I updated the nodemap hashes to fix the problem and changed the unit test so that it is tested past 128 nodemaps. I also fixed a bug in the idmap tree to fix an insertion/deletion problem. The patch is at http://review.whamcloud.com/#/c/10147. |
| Comment by Jodi Levi (Inactive) [ 07/Oct/14 ] |
|
Patch landed to Master. |