From 0a1e922f3d8eca4e852be57124ec1bcafaadb289 Mon Sep 17 00:00:00 2001 From: Konstantin V Shvachko Date: Mon, 16 Jul 2018 18:20:24 -0700 Subject: [PATCH] Fix potential FSImage corruption. Contributed by Ekanth Sethuramalingam & Arpit Agarwal. --- .../server/namenode/AclEntryStatusFormat.java | 6 +- .../namenode/INodeWithAdditionalFields.java | 4 +- .../hdfs/server/namenode/XAttrFormat.java | 67 ++++++++++++------- 3 files changed, 49 insertions(+), 28 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/AclEntryStatusFormat.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/AclEntryStatusFormat.java index 82aa214bf74..2c5b23b07c1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/AclEntryStatusFormat.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/AclEntryStatusFormat.java @@ -38,7 +38,8 @@ import com.google.common.collect.ImmutableList; * [1:3) -- the type of the entry (AclEntryType)
* [3:6) -- the permission of the entry (FsAction)
* [6:7) -- A flag to indicate whether Named entry or not
- * [7:32) -- the name of the entry, which is an ID that points to a
+ * [7:8) -- Reserved
+ * [8:32) -- the name of the entry, which is an ID that points to a
* string in the StringTableSection.
*/ public enum AclEntryStatusFormat { @@ -47,7 +48,8 @@ public enum AclEntryStatusFormat { TYPE(SCOPE.BITS, 2), PERMISSION(TYPE.BITS, 3), NAMED_ENTRY_CHECK(PERMISSION.BITS, 1), - NAME(NAMED_ENTRY_CHECK.BITS, 25); + RESERVED(NAMED_ENTRY_CHECK.BITS, 1), + NAME(RESERVED.BITS, 24); private final LongBitFormat BITS; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeWithAdditionalFields.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeWithAdditionalFields.java index 9adcc3eb63c..84d99e483ee 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeWithAdditionalFields.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeWithAdditionalFields.java @@ -35,8 +35,8 @@ public abstract class INodeWithAdditionalFields extends INode implements LinkedElement { enum PermissionStatusFormat { MODE(null, 16), - GROUP(MODE.BITS, 25), - USER(GROUP.BITS, 23); + GROUP(MODE.BITS, 24), + USER(GROUP.BITS, 24); final LongBitFormat BITS; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrFormat.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrFormat.java index 7e704d0226c..f9f06db8faf 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrFormat.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrFormat.java @@ -27,25 +27,56 @@ import org.apache.hadoop.hdfs.XAttrHelper; import com.google.common.base.Preconditions; import com.google.common.primitives.Ints; +import org.apache.hadoop.hdfs.util.LongBitFormat; /** * Class to pack XAttrs into byte[].
* For each XAttr:
* The first 4 bytes represents XAttr namespace and name
* [0:3) - XAttr namespace
- * [3:32) - The name of the entry, which is an ID that points to a + * [3:8) - Reserved
+ * [8:32) - The name of the entry, which is an ID that points to a * string in map
* The following two bytes represents the length of XAttr value
* The remaining bytes is the XAttr value
*/ class XAttrFormat { - private static final int XATTR_NAMESPACE_MASK = (1 << 3) - 1; - private static final int XATTR_NAMESPACE_OFFSET = 29; - private static final int XATTR_NAME_MASK = (1 << 29) - 1; - private static final int XATTR_NAME_ID_MAX = 1 << 29; + private enum XAttrStatusFormat { + + NAMESPACE(null, 3), + RESERVED(NAMESPACE.BITS, 5), + NAME(RESERVED.BITS, 24); + + private final LongBitFormat BITS; + + XAttrStatusFormat(LongBitFormat previous, int length) { + BITS = new LongBitFormat(name(), previous, length, 0); + } + + static XAttr.NameSpace getNamespace(int xattrStatus) { + int ordinal = (int) NAMESPACE.BITS.retrieve(xattrStatus); + return XAttr.NameSpace.values()[ordinal]; + } + + static String getName(int xattrStatus) { + int id = (int) NAME.BITS.retrieve(xattrStatus); + return XAttrStorage.getName(id); + } + + static int toInt(XAttr.NameSpace namespace, String name) { + long xattrStatusInt = 0; + + xattrStatusInt = NAMESPACE.BITS + .combine(namespace.ordinal(), xattrStatusInt); + int nid = XAttrStorage.getNameSerialNumber(name); + xattrStatusInt = NAME.BITS + .combine(nid, xattrStatusInt); + + return (int) xattrStatusInt; + } + } + private static final int XATTR_VALUE_LEN_MAX = 1 << 16; - private static final XAttr.NameSpace[] XATTR_NAMESPACE_VALUES = - XAttr.NameSpace.values(); /** * Unpack byte[] to XAttrs. @@ -64,10 +95,8 @@ class XAttrFormat { int v = Ints.fromBytes(attrs[i], attrs[i + 1], attrs[i + 2], attrs[i + 3]); i += 4; - int ns = (v >> XATTR_NAMESPACE_OFFSET) & XATTR_NAMESPACE_MASK; - int nid = v & XATTR_NAME_MASK; - builder.setNameSpace(XATTR_NAMESPACE_VALUES[ns]); - builder.setName(XAttrStorage.getName(nid)); + builder.setNameSpace(XAttrStatusFormat.getNamespace(v)); + builder.setName(XAttrStatusFormat.getName(v)); int vlen = ((0xff & attrs[i]) << 8) | (0xff & attrs[i + 1]); i += 2; if (vlen > 0) { @@ -100,10 +129,8 @@ class XAttrFormat { int v = Ints.fromBytes(attrs[i], attrs[i + 1], attrs[i + 2], attrs[i + 3]); i += 4; - int ns = (v >> XATTR_NAMESPACE_OFFSET) & XATTR_NAMESPACE_MASK; - int nid = v & XATTR_NAME_MASK; - XAttr.NameSpace namespace = XATTR_NAMESPACE_VALUES[ns]; - String name = XAttrStorage.getName(nid); + XAttr.NameSpace namespace = XAttrStatusFormat.getNamespace(v); + String name = XAttrStatusFormat.getName(v); int vlen = ((0xff & attrs[i]) << 8) | (0xff & attrs[i + 1]); i += 2; if (xAttr.getNameSpace() == namespace && @@ -134,15 +161,7 @@ class XAttrFormat { ByteArrayOutputStream out = new ByteArrayOutputStream(); try { for (XAttr a : xAttrs) { - int nsOrd = a.getNameSpace().ordinal(); - Preconditions.checkArgument(nsOrd < 8, "Too many namespaces."); - int nid = XAttrStorage.getNameSerialNumber(a.getName()); - Preconditions.checkArgument(nid < XATTR_NAME_ID_MAX, - "Too large serial number of the xattr name"); - - // big-endian - int v = ((nsOrd & XATTR_NAMESPACE_MASK) << XATTR_NAMESPACE_OFFSET) - | (nid & XATTR_NAME_MASK); + int v = XAttrStatusFormat.toInt(a.getNameSpace(), a.getName()); out.write(Ints.toByteArray(v)); int vlen = a.getValue() == null ? 0 : a.getValue().length; Preconditions.checkArgument(vlen < XATTR_VALUE_LEN_MAX,