Skip to content

Commit

Permalink
findNextPositivePowerOfTwo out of bounds
Browse files Browse the repository at this point in the history
Motivation:
Some usages of findNextPositivePowerOfTwo assume that bounds checking is taken care of by this method. However bounds checking is not taken care of by findNextPositivePowerOfTwo and instead assert statements are used to imply the caller has checked the bounds. This can lead to unexpected non power of 2 return values if the caller is not careful and thus invalidate any logic which depends upon a power of 2.

Modifications:
- Add a safeFindNextPositivePowerOfTwo method which will do runtime bounds checks and always return a power of 2

Result:
Fixes netty#5601
  • Loading branch information
Scottmitch authored and liuzhengyang committed Sep 10, 2017
1 parent 3abc913 commit 5676070
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 19 deletions.
2 changes: 1 addition & 1 deletion buffer/src/main/java/io/netty/buffer/PoolThreadCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ private abstract static class MemoryRegionCache<T> {
private int allocations;

MemoryRegionCache(int size, SizeClass sizeClass) {
this.size = MathUtil.findNextPositivePowerOfTwo(size);
this.size = MathUtil.safeFindNextPositivePowerOfTwo(size);
queue = PlatformDependent.newFixedMpscQueue(this.size);
this.sizeClass = sizeClass;
}
Expand Down
12 changes: 4 additions & 8 deletions common/src/main/java/io/netty/util/Recycler.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import java.util.WeakHashMap;
import java.util.concurrent.atomic.AtomicInteger;

import static io.netty.util.internal.MathUtil.findNextPositivePowerOfTwo;
import static io.netty.util.internal.MathUtil.safeFindNextPositivePowerOfTwo;
import static java.lang.Math.max;
import static java.lang.Math.min;

Expand Down Expand Up @@ -71,14 +71,13 @@ public void recycle(Object object) {
SystemPropertyUtil.getInt("io.netty.recycler.maxSharedCapacityFactor",
2));

LINK_CAPACITY = findNextPositivePowerOfTwo(
LINK_CAPACITY = safeFindNextPositivePowerOfTwo(
max(SystemPropertyUtil.getInt("io.netty.recycler.linkCapacity", 16), 16));

// By default we allow one push to a Recycler for each 8th try on handles that were never recycled before.
// This should help to slowly increase the capacity of the recycler while not be too sensitive to allocation
// bursts.
RATIO = min(findNextPositivePowerOfTwo(
max(SystemPropertyUtil.getInt("io.netty.recycler.ratio", 8), 2)), 0x40000000);
RATIO = safeFindNextPositivePowerOfTwo(SystemPropertyUtil.getInt("io.netty.recycler.ratio", 8));

if (logger.isDebugEnabled()) {
if (DEFAULT_MAX_CAPACITY == 0) {
Expand Down Expand Up @@ -121,10 +120,7 @@ protected Recycler(int maxCapacity, int maxSharedCapacityFactor) {
}

protected Recycler(int maxCapacity, int maxSharedCapacityFactor, int ratio) {
if (ratio > 0x40000000) {
throw new IllegalArgumentException(ratio + ": " + ratio + " (expected: < 0x40000000)");
}
ratioMask = findNextPositivePowerOfTwo(ratio) - 1;
ratioMask = safeFindNextPositivePowerOfTwo(ratio) - 1;
if (maxCapacity <= 0) {
this.maxCapacity = 0;
this.maxSharedCapacityFactor = 1;
Expand Down
5 changes: 1 addition & 4 deletions common/src/main/java/io/netty/util/ResourceLeakDetector.java
Original file line number Diff line number Diff line change
Expand Up @@ -193,15 +193,12 @@ public ResourceLeakDetector(String resourceType, int samplingInterval, long maxA
if (resourceType == null) {
throw new NullPointerException("resourceType");
}
if (samplingInterval <= 0) {
throw new IllegalArgumentException("samplingInterval: " + samplingInterval + " (expected: 1+)");
}
if (maxActive <= 0) {
throw new IllegalArgumentException("maxActive: " + maxActive + " (expected: 1+)");
}

this.resourceType = resourceType;
this.samplingInterval = MathUtil.findNextPositivePowerOfTwo(samplingInterval);
this.samplingInterval = MathUtil.safeFindNextPositivePowerOfTwo(samplingInterval);
// samplingInterval is a power of two so we calculate a mask that we can use to
// check if we need to do any leak detection or not.
mask = this.samplingInterval - 1;
Expand Down
18 changes: 17 additions & 1 deletion common/src/main/java/io/netty/util/internal/MathUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ private MathUtil() {
/**
* Fast method of finding the next power of 2 greater than or equal to the supplied value.
*
* If the value is {@code <= 0} then 1 will be returned.
* <p>If the value is {@code <= 0} then 1 will be returned.
* This method is not suitable for {@link Integer#MIN_VALUE} or numbers greater than 2^30.
*
* @param value from which to search for next power of 2
Expand All @@ -36,6 +36,22 @@ public static int findNextPositivePowerOfTwo(final int value) {
return 1 << (32 - Integer.numberOfLeadingZeros(value - 1));
}

/**
* Fast method of finding the next power of 2 greater than or equal to the supplied value.
* <p>This method will do runtime bounds checking and call {@link #findNextPositivePowerOfTwo(int)} if within a
* valid range.
* @param value from which to search for next power of 2
* @return The next power of 2 or the value itself if it is a power of 2.
* <p>Special cases for return values are as follows:
* <ul>
* <li>{@code <= 0} -> 1</li>
* <li>{@code >= 2^30} -> 2^30</li>
* </ul>
*/
public static int safeFindNextPositivePowerOfTwo(final int value) {
return value <= 0 ? 1 : value >= 0x40000000 ? 0x40000000 : findNextPositivePowerOfTwo(value);
}

/**
* Determine if the requested {@code index} and {@code length} will fit within {@code capacity}.
* @param index The starting index.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

package io.netty.util.collection;

import static io.netty.util.internal.MathUtil.findNextPositivePowerOfTwo;
import static io.netty.util.internal.MathUtil.safeFindNextPositivePowerOfTwo;

import java.util.AbstractCollection;
import java.util.AbstractSet;
Expand Down Expand Up @@ -77,9 +77,6 @@ public class @K@ObjectHashMap<V> implements @K@ObjectMap<V> {
}

public @K@ObjectHashMap(int initialCapacity, float loadFactor) {
if (initialCapacity < 1) {
throw new IllegalArgumentException("initialCapacity must be >= 1");
}
if (loadFactor <= 0.0f || loadFactor > 1.0f) {
// Cannot exceed 1 because we can never store more than capacity elements;
// using a bigger loadFactor would trigger rehashing before the desired load is reached.
Expand All @@ -89,7 +86,7 @@ public class @K@ObjectHashMap<V> implements @K@ObjectMap<V> {
this.loadFactor = loadFactor;

// Adjust the initial capacity if necessary.
int capacity = findNextPositivePowerOfTwo(initialCapacity);
int capacity = safeFindNextPositivePowerOfTwo(initialCapacity);
mask = capacity - 1;

// Allocate the arrays.
Expand Down

0 comments on commit 5676070

Please sign in to comment.