Fix bug 28982. Call to JS_ClearScope in property setter (e.g. window.location = "foo")

was causing defered calls to js_FreeSlot to do wild pointer writes into slots that
were no longer owned by the object. Also this improves the fix to 14462 (see note
in 28982 from brendan). r=brendan@mozilla.org a=jar@netscape.com
This commit is contained in:
jband%netscape.com 2000-02-26 23:47:33 +00:00
parent f28f63a5d7
commit 417094058a
4 changed files with 50 additions and 15 deletions

View File

@ -2094,6 +2094,7 @@ JS_ClearScope(JSContext *cx, JSObject *obj)
{
JSObjectMap *map;
JSScope *scope;
uint32 i;
CHECK_REQUEST(cx);
/* XXXbe push this into jsobj.c or jsscope.c */
@ -2104,8 +2105,10 @@ JS_ClearScope(JSContext *cx, JSObject *obj)
scope->ops->clear(cx, scope);
}
/* Reset freeslot so we're consistent. */
/* Clear slot values and reset freeslot so we're consistent. */
map->freeslot = JSSLOT_FREE(OBJ_GET_CLASS(cx, obj));
for (i = map->nslots-1; i >= map->freeslot; --i)
obj->slots[i] = JSVAL_VOID;
JS_UNLOCK_OBJ(cx, obj);
}

View File

@ -1445,6 +1445,7 @@ js_FreeSlot(JSContext *cx, JSObject *obj, uint32 slot)
size_t nbytes;
jsval *newslots;
OBJ_CHECK_SLOT(obj,slot);
obj->slots[slot] = JSVAL_VOID;
map = obj->map;
if (map->freeslot == slot + 1)
@ -1801,6 +1802,7 @@ js_GetProperty(JSContext *cx, JSObject *obj, jsid id, jsval *vp)
{
JSObject *obj2;
JSScopeProperty *sprop;
JSScope *scope;
jsint slot;
if (!js_LookupProperty(cx, obj, id, &obj2, (JSProperty **)&sprop))
@ -1831,19 +1833,25 @@ js_GetProperty(JSContext *cx, JSObject *obj, jsid id, jsval *vp)
}
/* Unlock obj2 before calling getter, relock after to avoid deadlock. */
scope = OBJ_SCOPE(obj2);
slot = sprop->slot;
*vp = LOCKED_OBJ_GET_SLOT(obj2, slot);
JS_UNLOCK_OBJ(cx, obj2);
#ifndef JS_THREADSAFE
JS_ATOMIC_ADDREF(&sprop->nrefs, 1);
#endif
JS_UNLOCK_SCOPE(cx, scope);
if (!SPROP_GET(cx, sprop, obj, obj2, vp)) {
JS_LOCK_OBJ(cx, obj2);
OBJ_DROP_PROPERTY(cx, obj2, (JSProperty *)sprop);
JS_LOCK_OBJ_VOID(cx, obj2, js_DropScopeProperty(cx, scope, sprop));
return JS_FALSE;
}
JS_LOCK_OBJ(cx, obj2);
LOCKED_OBJ_SET_SLOT(obj2, slot, *vp);
PROPERTY_CACHE_FILL(cx, &cx->runtime->propertyCache, obj2, id,
(JSProperty *)sprop);
OBJ_DROP_PROPERTY(cx, obj2, (JSProperty *)sprop);
JS_LOCK_SCOPE(cx, scope);
sprop = js_DropScopeProperty(cx, scope, sprop);
if (sprop && SPROP_HAS_VALID_SLOT(sprop)) {
LOCKED_OBJ_SET_SLOT(obj2, slot, *vp);
PROPERTY_CACHE_FILL(cx, &cx->runtime->propertyCache, obj2, id,
(JSProperty *)sprop);
}
JS_UNLOCK_SCOPE(cx, scope);
return JS_TRUE;
}
@ -2056,7 +2064,7 @@ read_only:
JS_LOCK_OBJ(cx, obj);
sprop = js_DropScopeProperty(cx, scope, sprop);
if (!sprop) {
if (!sprop || !SPROP_HAS_VALID_SLOT(sprop)) {
/* Lost a race with someone who deleted sprop. */
JS_UNLOCK_OBJ(cx, obj);
return JS_TRUE;

View File

@ -205,6 +205,20 @@ js_hash_scope_remove(JSContext *cx, JSScope *scope, jsid id)
return JS_HashTableRemove(table, (const void *)id);
}
JS_STATIC_DLL_CALLBACK(intN)
js_hash_scope_slot_invalidator(JSHashEntry *he, intN i, void *arg)
{
JSSymbol *sym = (JSSymbol *) he;
JSScopeProperty *sprop;
if (sym) {
sprop = (JSScopeProperty *) sym->entry.value;
if (sprop)
sprop->slot = SPROP_INVALID_SLOT;
}
return HT_ENUMERATE_NEXT;
}
/* Forward declaration for use by js_hash_scope_clear(). */
extern JS_FRIEND_DATA(JSScopeOps) js_list_scope_ops;
@ -217,6 +231,7 @@ js_hash_scope_clear(JSContext *cx, JSScope *scope)
JS_ASSERT(JS_IS_SCOPE_LOCKED(scope));
priv = (JSScopePrivate *) table->allocPriv;
priv->context = cx;
JS_HashTableEnumerateEntries(table, js_hash_scope_slot_invalidator, NULL);
JS_HashTableDestroy(table);
JS_free(cx, priv);
scope->ops = &js_list_scope_ops;
@ -340,13 +355,17 @@ js_list_scope_clear(JSContext *cx, JSScope *scope)
{
JSSymbol *sym;
JSScopePrivate priv;
JSScopeProperty *sprop;
JS_ASSERT(JS_IS_SCOPE_LOCKED(scope));
while ((sym = (JSSymbol *) scope->data) != NULL) {
scope->data = sym->entry.next;
priv.context = cx;
priv.scope = scope;
js_free_symbol(&priv, &sym->entry, HT_FREE_ENTRY);
scope->data = sym->entry.next;
priv.context = cx;
priv.scope = scope;
sprop = (JSScopeProperty *) sym->entry.value;
if (sprop)
sprop->slot = SPROP_INVALID_SLOT;
js_free_symbol(&priv, &sym->entry, HT_FREE_ENTRY);
}
}
@ -497,7 +516,9 @@ js_DestroyScopeProperty(JSContext *cx, JSScope *scope, JSScopeProperty *sprop)
if (scope) {
JS_ASSERT(JS_IS_SCOPE_LOCKED(scope));
if (scope->object) {
js_FreeSlot(cx, scope->object, sprop->slot);
JS_ASSERT(sprop);
if (SPROP_HAS_VALID_SLOT(sprop))
js_FreeSlot(cx, scope->object, sprop->slot);
*sprop->prevp = sprop->next;
if (sprop->next)
sprop->next->prevp = sprop->prevp;

View File

@ -82,6 +82,9 @@ struct JSScope {
#define SPROP_GETTER(sprop,obj) SPROP_GETTER_SCOPE(sprop, OBJ_SCOPE(obj))
#define SPROP_SETTER(sprop,obj) SPROP_SETTER_SCOPE(sprop, OBJ_SCOPE(obj))
#define SPROP_INVALID_SLOT 0xffffffff
#define SPROP_HAS_VALID_SLOT(_s) ((_s)->slot != SPROP_INVALID_SLOT)
#ifdef JS_DOUBLE_HASHING
struct JSScopeProperty {