diff mbox series

[RFC,v5,11/57] objtool: Abstract alternative special case handling

Message ID 20200109160300.26150-12-jthierry@redhat.com (mailing list archive)
State New, archived
Headers show
Series objtool: Add support for arm64 | expand

Commit Message

Julien Thierry Jan. 9, 2020, 4:02 p.m. UTC 
Some alternatives associated with a specific feature need to be treated
in a special way. Since the features and how to treat them vary from one
architecture to another, move the special case handling to arch specific
code.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
 tools/objtool/arch/x86/Build                  |  1 +
 tools/objtool/arch/x86/arch_special.c         | 34 +++++++++++++++++++
 tools/objtool/arch/x86/include/arch_special.h |  5 +++
 tools/objtool/special.c                       | 25 +-------------
 tools/objtool/special.h                       |  8 +++++
 5 files changed, 49 insertions(+), 24 deletions(-)
 create mode 100644 tools/objtool/arch/x86/arch_special.c

Comments

Peter Zijlstra Jan. 20, 2020, 2:54 p.m. UTC | #1 
On Thu, Jan 09, 2020 at 04:02:14PM +0000, Julien Thierry wrote:
> diff --git a/tools/objtool/arch/x86/arch_special.c b/tools/objtool/arch/x86/arch_special.c
> new file mode 100644
> index 000000000000..6dba31f419d0
> --- /dev/null
> +++ b/tools/objtool/arch/x86/arch_special.c
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#include "../../special.h"
> +#include "../../builtin.h"
> +
> +void arch_handle_alternative(unsigned short feature, struct special_alt *alt)
> +{
> +	/*
> +	 * If UACCESS validation is enabled; force that alternative;
> +	 * otherwise force it the other way.
> +	 *
> +	 * What we want to avoid is having both the original and the
> +	 * alternative code flow at the same time, in that case we can
> +	 * find paths that see the STAC but take the NOP instead of
> +	 * CLAC and the other way around.
> +	 */

That comment ^,

> +	switch (feature) {
> +	case X86_FEATURE_SMAP:

goes here >

> +		if (uaccess)
> +			alt->skip_orig = true;
> +		else
> +			alt->skip_alt = true;
> +		break;

> +	case X86_FEATURE_POPCNT:
> +		/*
> +		 * It has been requested that we don't validate the !POPCNT
> +		 * feature path which is a "very very small percentage of
> +		 * machines".
> +		 */
> +		alt->skip_orig = true;
> +		break;
> +	default:
> +		break;
> +	}
> +}
Julien Thierry Jan. 23, 2020, 11:45 a.m. UTC | #2 
On 1/20/20 2:54 PM, Peter Zijlstra wrote:
> On Thu, Jan 09, 2020 at 04:02:14PM +0000, Julien Thierry wrote:
>> diff --git a/tools/objtool/arch/x86/arch_special.c b/tools/objtool/arch/x86/arch_special.c
>> new file mode 100644
>> index 000000000000..6dba31f419d0
>> --- /dev/null
>> +++ b/tools/objtool/arch/x86/arch_special.c
>> @@ -0,0 +1,34 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +#include "../../special.h"
>> +#include "../../builtin.h"
>> +
>> +void arch_handle_alternative(unsigned short feature, struct special_alt *alt)
>> +{
>> +	/*
>> +	 * If UACCESS validation is enabled; force that alternative;
>> +	 * otherwise force it the other way.
>> +	 *
>> +	 * What we want to avoid is having both the original and the
>> +	 * alternative code flow at the same time, in that case we can
>> +	 * find paths that see the STAC but take the NOP instead of
>> +	 * CLAC and the other way around.
>> +	 */
> 
> That comment ^,
> 
>> +	switch (feature) {
>> +	case X86_FEATURE_SMAP:
> 
> goes here >
> 

Good catch, I'll fix that.

>> +		if (uaccess)
>> +			alt->skip_orig = true;
>> +		else
>> +			alt->skip_alt = true;
>> +		break;
> 
>> +	case X86_FEATURE_POPCNT:
>> +		/*
>> +		 * It has been requested that we don't validate the !POPCNT
>> +		 * feature path which is a "very very small percentage of
>> +		 * machines".
>> +		 */
>> +		alt->skip_orig = true;
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +}
> 

Thanks,
diff mbox series

Patch

diff --git a/tools/objtool/arch/x86/Build b/tools/objtool/arch/x86/Build
index e43fd6fa0ee1..971f9fa90a3c 100644
--- a/tools/objtool/arch/x86/Build
+++ b/tools/objtool/arch/x86/Build
@@ -1,3 +1,4 @@ 
+objtool-y += arch_special.o
 objtool-y += decode.o
 objtool-y += orc_dump.o
 objtool-y += orc_gen.o
diff --git a/tools/objtool/arch/x86/arch_special.c b/tools/objtool/arch/x86/arch_special.c
new file mode 100644
index 000000000000..6dba31f419d0
--- /dev/null
+++ b/tools/objtool/arch/x86/arch_special.c
@@ -0,0 +1,34 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include "../../special.h"
+#include "../../builtin.h"
+
+void arch_handle_alternative(unsigned short feature, struct special_alt *alt)
+{
+	/*
+	 * If UACCESS validation is enabled; force that alternative;
+	 * otherwise force it the other way.
+	 *
+	 * What we want to avoid is having both the original and the
+	 * alternative code flow at the same time, in that case we can
+	 * find paths that see the STAC but take the NOP instead of
+	 * CLAC and the other way around.
+	 */
+	switch (feature) {
+	case X86_FEATURE_SMAP:
+		if (uaccess)
+			alt->skip_orig = true;
+		else
+			alt->skip_alt = true;
+		break;
+	case X86_FEATURE_POPCNT:
+		/*
+		 * It has been requested that we don't validate the !POPCNT
+		 * feature path which is a "very very small percentage of
+		 * machines".
+		 */
+		alt->skip_orig = true;
+		break;
+	default:
+		break;
+	}
+}
diff --git a/tools/objtool/arch/x86/include/arch_special.h b/tools/objtool/arch/x86/include/arch_special.h
index 426178d504a8..3ab2dc32424b 100644
--- a/tools/objtool/arch/x86/include/arch_special.h
+++ b/tools/objtool/arch/x86/include/arch_special.h
@@ -20,4 +20,9 @@ 
 #define X86_FEATURE_POPCNT (4 * 32 + 23)
 #define X86_FEATURE_SMAP   (9 * 32 + 20)
 
+struct special_alt;
+
+#define arch_handle_alternative arch_handle_alternative
+void arch_handle_alternative(unsigned short feature, struct special_alt *alt);
+
 #endif /* _X86_ARCH_SPECIAL_H */
diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index b8ccee1b5382..67461b25e649 100644
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -75,30 +75,7 @@  static int get_alt_entry(struct elf *elf, struct special_entry *entry,
 
 		feature = *(unsigned short *)(sec->data->d_buf + offset +
 					      entry->feature);
-
-		/*
-		 * It has been requested that we don't validate the !POPCNT
-		 * feature path which is a "very very small percentage of
-		 * machines".
-		 */
-		if (feature == X86_FEATURE_POPCNT)
-			alt->skip_orig = true;
-
-		/*
-		 * If UACCESS validation is enabled; force that alternative;
-		 * otherwise force it the other way.
-		 *
-		 * What we want to avoid is having both the original and the
-		 * alternative code flow at the same time, in that case we can
-		 * find paths that see the STAC but take the NOP instead of
-		 * CLAC and the other way around.
-		 */
-		if (feature == X86_FEATURE_SMAP) {
-			if (uaccess)
-				alt->skip_orig = true;
-			else
-				alt->skip_alt = true;
-		}
+		arch_handle_alternative(feature, alt);
 	}
 
 	orig_rela = find_rela_by_dest(sec, offset + entry->orig);
diff --git a/tools/objtool/special.h b/tools/objtool/special.h
index 35061530e46e..738a05bc6d3a 100644
--- a/tools/objtool/special.h
+++ b/tools/objtool/special.h
@@ -8,6 +8,7 @@ 
 
 #include <stdbool.h>
 #include "elf.h"
+#include "arch_special.h"
 
 struct special_alt {
 	struct list_head list;
@@ -28,4 +29,11 @@  struct special_alt {
 
 int special_get_alts(struct elf *elf, struct list_head *alts);
 
+#ifndef arch_handle_alternative
+static inline void arch_handle_alternative(unsigned short feature,
+					   struct special_alt *alt)
+{
+}
+#endif
+
 #endif /* _SPECIAL_H */