The patch titled idr: fix idr corruption when id crosses a layer boundary has been added to the -mm tree. Its filename is idr-fix-idr-corruption-when-id-crosses-a-layer-boundary.patch Before you just go and hit "reply", please: a) Consider who else should be cc'ed b) Prefer to cc a suitable mailing list as well c) Ideally: find the original patch on the mailing list and do a reply-to-all to that, adding suitable additional cc's *** Remember to use Documentation/SubmitChecklist when testing your code *** See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find out what to do about this The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/ ------------------------------------------------------ Subject: idr: fix idr corruption when id crosses a layer boundary From: Eric Paris <eparis@xxxxxxxxxx> inotify has for months been been having problems keeping the fsnotify view of its objects in sync with the idr view of its objects. I discovered this was only ever the case with the 4096th object created. What actually happened is that inotify would try to do the following operation: idr_get_new_above(idr, NULL, 4095, &id); and would get id=4095. idr_get_new_above(idr, NULL, 4095, &id); and would get id=4096. Notice I used 4095 both times. Everything seems fine. We would then do: idr_find(idr, 4095); and would get the entry for 4095. idr_find(idr, 4096); and would get NULL!! WHOOPS! idr_remove(idr, 4096) strangely enough didn't blow up. What I discovered was that when adding an id >= 4096 the idr needed to grow another layer. This was normally dealt with in idr_get_empty_slot() if we found that the id was larger than the idr could hold. The problem was that I was passing 4095 which appeared to be small enough to fit, but in sub_alloc() we found that 4095 was taken and bumped it up to 4096. I added a test in sub_alloc() which kicks back out if the id is changed to be larger than fits in the idr. I readily admit that I don't understand the other part of that test or how we 'get back to the top layer.' I wonder if that is not what was supposed to deal with this. This certainly needs some review, but it fixes my problem and should finally silence the months and months old inotify complaints people's machines have been spewing... #include <linux/idr.h> #include <linux/kernel.h> #include <linux/module.h> static DEFINE_IDR(test_idr); int init_module(void) { int ret, forty95, forty96; void *addr; /* add 2 entries both with 4095 as the start address */ again1: if (!idr_pre_get(&test_idr, GFP_KERNEL)) return -ENOMEM; ret = idr_get_new_above(&test_idr, (void *)4095, 4095, &forty95); if (ret) { if (ret == -EAGAIN) goto again1; return ret; } if (forty95 != 4095) printk(KERN_ERR "hmmm, forty95=%d\n", forty95); again2: if (!idr_pre_get(&test_idr, GFP_KERNEL)) return -ENOMEM; ret = idr_get_new_above(&test_idr, (void *)4096, 4095, &forty96); if (ret) { if (ret == -EAGAIN) goto again2; return ret; } if (forty96 != 4096) printk(KERN_ERR "hmmm, forty96=%d\n", forty96); /* try to find the 2 entries, noticing that 4096 broke */ addr = idr_find(&test_idr, forty95); if ((int)addr != forty95) printk(KERN_ERR "hmmm, after find forty95=%d addr=%d\n", forty95, (int)addr); addr = idr_find(&test_idr, forty96); if ((int)addr != forty96) printk(KERN_ERR "hmmm, after find forty96=%d addr=%d\n", forty96, (int)addr); /* really weird, the entry which should be at 4096 is actually at 0!! */ addr = idr_find(&test_idr, 0); if ((int)addr) printk(KERN_ERR "found an entry at id=0 for addr=%d\n", (int)addr); idr_remove(&test_idr, forty95); idr_remove(&test_idr, forty96); return 0; } void cleanup_module(void) { } MODULE_AUTHOR("Eric Paris <eparis@xxxxxxxxxx>"); MODULE_DESCRIPTION("Simple idr test"); MODULE_LICENSE("GPL"); Signed-off-by: Eric Paris <eparis@xxxxxxxxxx> Cc: Tejun Heo <htejun@xxxxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> --- lib/idr.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff -puN lib/idr.c~idr-fix-idr-corruption-when-id-crosses-a-layer-boundary lib/idr.c --- a/lib/idr.c~idr-fix-idr-corruption-when-id-crosses-a-layer-boundary +++ a/lib/idr.c @@ -155,8 +155,12 @@ static int sub_alloc(struct idr *idp, in oid = id; id = (id | ((1 << (IDR_BITS * l)) - 1)) + 1; - /* if already at the top layer, we need to grow */ - if (!(p = pa[l])) { + /* + * if already at the top layer or if we just rounded id + * to be so large the idp doesn't have enough layers, + * we need to grow. + */ + if (!(p = pa[l]) || (id >= (1 << (idp->layers * IDR_BITS)))) { *starting_id = id; return IDR_NEED_TO_GROW; } @@ -260,7 +264,7 @@ build_up: static int idr_get_new_above_int(struct idr *idp, void *ptr, int starting_id) { - struct idr_layer *pa[MAX_LEVEL]; + struct idr_layer *pa[MAX_LEVEL] = { NULL, }; int id; id = idr_get_empty_slot(idp, starting_id, pa); _ Patches currently in -mm which might be from eparis@xxxxxxxxxx are linux-next.patch idr-fix-idr-corruption-when-id-crosses-a-layer-boundary.patch -- To unsubscribe from this list: send the line "unsubscribe mm-commits" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html