2021-11-23 07:34:26

by Akira Kawata

[permalink] [raw]
Subject: [PATCH v2] fs/binfmt_elf: Fix AT_PHDR for unusual ELF files

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=197921

As pointed out in the discussion of buglink, we cannot calculate AT_PHDR
as the sum of load_addr and exec->e_phoff. This is because exec->e_phoff
is the offset of PHDRs in the file and the address of PHDRs in the
memory may differ from it. This patch fixes the bug by calculating the
address of program headers from PT_LOADs directly.

Signed-off-by: Akira Kawata <[email protected]>
---
Changes in v2:
- Remove unused load_addr from create_elf_tables.
- Improve the commit message.

fs/binfmt_elf.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index f8c7f26f1fbb..af8313e36665 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -170,8 +170,8 @@ static int padzero(unsigned long elf_bss)

static int
create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec,
- unsigned long load_addr, unsigned long interp_load_addr,
- unsigned long e_entry)
+ unsigned long interp_load_addr,
+ unsigned long e_entry, unsigned long phdr_addr)
{
struct mm_struct *mm = current->mm;
unsigned long p = bprm->p;
@@ -257,7 +257,7 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec,
NEW_AUX_ENT(AT_HWCAP, ELF_HWCAP);
NEW_AUX_ENT(AT_PAGESZ, ELF_EXEC_PAGESIZE);
NEW_AUX_ENT(AT_CLKTCK, CLOCKS_PER_SEC);
- NEW_AUX_ENT(AT_PHDR, load_addr + exec->e_phoff);
+ NEW_AUX_ENT(AT_PHDR, phdr_addr);
NEW_AUX_ENT(AT_PHENT, sizeof(struct elf_phdr));
NEW_AUX_ENT(AT_PHNUM, exec->e_phnum);
NEW_AUX_ENT(AT_BASE, interp_load_addr);
@@ -823,7 +823,7 @@ static int parse_elf_properties(struct file *f, const struct elf_phdr *phdr,
static int load_elf_binary(struct linux_binprm *bprm)
{
struct file *interpreter = NULL; /* to shut gcc up */
- unsigned long load_addr = 0, load_bias = 0;
+ unsigned long load_addr = 0, load_bias = 0, phdr_addr = 0;
int load_addr_set = 0;
unsigned long error;
struct elf_phdr *elf_ppnt, *elf_phdata, *interp_elf_phdata = NULL;
@@ -1169,6 +1169,13 @@ static int load_elf_binary(struct linux_binprm *bprm)
reloc_func_desc = load_bias;
}
}
+
+ if (elf_ppnt->p_offset <= elf_ex->e_phoff &&
+ elf_ex->e_phoff < elf_ppnt->p_offset + elf_ppnt->p_filesz) {
+ phdr_addr = elf_ex->e_phoff - elf_ppnt->p_offset +
+ elf_ppnt->p_vaddr;
+ }
+
k = elf_ppnt->p_vaddr;
if ((elf_ppnt->p_flags & PF_X) && k < start_code)
start_code = k;
@@ -1204,6 +1211,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
}

e_entry = elf_ex->e_entry + load_bias;
+ phdr_addr += load_bias;
elf_bss += load_bias;
elf_brk += load_bias;
start_code += load_bias;
@@ -1267,8 +1275,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
goto out;
#endif /* ARCH_HAS_SETUP_ADDITIONAL_PAGES */

- retval = create_elf_tables(bprm, elf_ex,
- load_addr, interp_load_addr, e_entry);
+ retval = create_elf_tables(bprm, elf_ex, interp_load_addr,
+ e_entry, phdr_addr);
if (retval < 0)
goto out;

--
2.25.1



2021-11-24 00:14:12

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2] fs/binfmt_elf: Fix AT_PHDR for unusual ELF files

Hi Akira,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on hnaz-mm/master]
[also build test WARNING on kees/for-next/pstore linus/master v5.16-rc2 next-20211123]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Akira-Kawata/fs-binfmt_elf-Fix-AT_PHDR-for-unusual-ELF-files/20211123-153459
base: https://github.com/hnaz/linux-mm master
config: i386-randconfig-a012-20211123 (https://download.01.org/0day-ci/archive/20211124/[email protected]/config.gz)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 49e3838145dff1ec91c2e67a2cb562775c8d2a08)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/8e8533fa0fdbe61a557de9268ea7091a75aebe81
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Akira-Kawata/fs-binfmt_elf-Fix-AT_PHDR-for-unusual-ELF-files/20211123-153459
git checkout 8e8533fa0fdbe61a557de9268ea7091a75aebe81
# save the config file to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> fs/binfmt_elf.c:825:16: warning: variable 'load_addr' set but not used [-Wunused-but-set-variable]
unsigned long load_addr = 0, load_bias = 0, phdr_addr = 0;
^
1 warning generated.


vim +/load_addr +825 fs/binfmt_elf.c

821
822 static int load_elf_binary(struct linux_binprm *bprm)
823 {
824 struct file *interpreter = NULL; /* to shut gcc up */
> 825 unsigned long load_addr = 0, load_bias = 0, phdr_addr = 0;
826 int load_addr_set = 0;
827 unsigned long error;
828 struct elf_phdr *elf_ppnt, *elf_phdata, *interp_elf_phdata = NULL;
829 struct elf_phdr *elf_property_phdata = NULL;
830 unsigned long elf_bss, elf_brk;
831 int bss_prot = 0;
832 int retval, i;
833 unsigned long elf_entry;
834 unsigned long e_entry;
835 unsigned long interp_load_addr = 0;
836 unsigned long start_code, end_code, start_data, end_data;
837 unsigned long reloc_func_desc __maybe_unused = 0;
838 int executable_stack = EXSTACK_DEFAULT;
839 struct elfhdr *elf_ex = (struct elfhdr *)bprm->buf;
840 struct elfhdr *interp_elf_ex = NULL;
841 struct arch_elf_state arch_state = INIT_ARCH_ELF_STATE;
842 struct mm_struct *mm;
843 struct pt_regs *regs;
844
845 retval = -ENOEXEC;
846 /* First of all, some simple consistency checks */
847 if (memcmp(elf_ex->e_ident, ELFMAG, SELFMAG) != 0)
848 goto out;
849
850 if (elf_ex->e_type != ET_EXEC && elf_ex->e_type != ET_DYN)
851 goto out;
852 if (!elf_check_arch(elf_ex))
853 goto out;
854 if (elf_check_fdpic(elf_ex))
855 goto out;
856 if (!bprm->file->f_op->mmap)
857 goto out;
858
859 elf_phdata = load_elf_phdrs(elf_ex, bprm->file);
860 if (!elf_phdata)
861 goto out;
862
863 elf_ppnt = elf_phdata;
864 for (i = 0; i < elf_ex->e_phnum; i++, elf_ppnt++) {
865 char *elf_interpreter;
866
867 if (elf_ppnt->p_type == PT_GNU_PROPERTY) {
868 elf_property_phdata = elf_ppnt;
869 continue;
870 }
871
872 if (elf_ppnt->p_type != PT_INTERP)
873 continue;
874
875 /*
876 * This is the program interpreter used for shared libraries -
877 * for now assume that this is an a.out format binary.
878 */
879 retval = -ENOEXEC;
880 if (elf_ppnt->p_filesz > PATH_MAX || elf_ppnt->p_filesz < 2)
881 goto out_free_ph;
882
883 retval = -ENOMEM;
884 elf_interpreter = kmalloc(elf_ppnt->p_filesz, GFP_KERNEL);
885 if (!elf_interpreter)
886 goto out_free_ph;
887
888 retval = elf_read(bprm->file, elf_interpreter, elf_ppnt->p_filesz,
889 elf_ppnt->p_offset);
890 if (retval < 0)
891 goto out_free_interp;
892 /* make sure path is NULL terminated */
893 retval = -ENOEXEC;
894 if (elf_interpreter[elf_ppnt->p_filesz - 1] != '\0')
895 goto out_free_interp;
896
897 interpreter = open_exec(elf_interpreter);
898 kfree(elf_interpreter);
899 retval = PTR_ERR(interpreter);
900 if (IS_ERR(interpreter))
901 goto out_free_ph;
902
903 /*
904 * If the binary is not readable then enforce mm->dumpable = 0
905 * regardless of the interpreter's permissions.
906 */
907 would_dump(bprm, interpreter);
908
909 interp_elf_ex = kmalloc(sizeof(*interp_elf_ex), GFP_KERNEL);
910 if (!interp_elf_ex) {
911 retval = -ENOMEM;
912 goto out_free_ph;
913 }
914
915 /* Get the exec headers */
916 retval = elf_read(interpreter, interp_elf_ex,
917 sizeof(*interp_elf_ex), 0);
918 if (retval < 0)
919 goto out_free_dentry;
920
921 break;
922
923 out_free_interp:
924 kfree(elf_interpreter);
925 goto out_free_ph;
926 }
927
928 elf_ppnt = elf_phdata;
929 for (i = 0; i < elf_ex->e_phnum; i++, elf_ppnt++)
930 switch (elf_ppnt->p_type) {
931 case PT_GNU_STACK:
932 if (elf_ppnt->p_flags & PF_X)
933 executable_stack = EXSTACK_ENABLE_X;
934 else
935 executable_stack = EXSTACK_DISABLE_X;
936 break;
937
938 case PT_LOPROC ... PT_HIPROC:
939 retval = arch_elf_pt_proc(elf_ex, elf_ppnt,
940 bprm->file, false,
941 &arch_state);
942 if (retval)
943 goto out_free_dentry;
944 break;
945 }
946
947 /* Some simple consistency checks for the interpreter */
948 if (interpreter) {
949 retval = -ELIBBAD;
950 /* Not an ELF interpreter */
951 if (memcmp(interp_elf_ex->e_ident, ELFMAG, SELFMAG) != 0)
952 goto out_free_dentry;
953 /* Verify the interpreter has a valid arch */
954 if (!elf_check_arch(interp_elf_ex) ||
955 elf_check_fdpic(interp_elf_ex))
956 goto out_free_dentry;
957
958 /* Load the interpreter program headers */
959 interp_elf_phdata = load_elf_phdrs(interp_elf_ex,
960 interpreter);
961 if (!interp_elf_phdata)
962 goto out_free_dentry;
963
964 /* Pass PT_LOPROC..PT_HIPROC headers to arch code */
965 elf_property_phdata = NULL;
966 elf_ppnt = interp_elf_phdata;
967 for (i = 0; i < interp_elf_ex->e_phnum; i++, elf_ppnt++)
968 switch (elf_ppnt->p_type) {
969 case PT_GNU_PROPERTY:
970 elf_property_phdata = elf_ppnt;
971 break;
972
973 case PT_LOPROC ... PT_HIPROC:
974 retval = arch_elf_pt_proc(interp_elf_ex,
975 elf_ppnt, interpreter,
976 true, &arch_state);
977 if (retval)
978 goto out_free_dentry;
979 break;
980 }
981 }
982
983 retval = parse_elf_properties(interpreter ?: bprm->file,
984 elf_property_phdata, &arch_state);
985 if (retval)
986 goto out_free_dentry;
987
988 /*
989 * Allow arch code to reject the ELF at this point, whilst it's
990 * still possible to return an error to the code that invoked
991 * the exec syscall.
992 */
993 retval = arch_check_elf(elf_ex,
994 !!interpreter, interp_elf_ex,
995 &arch_state);
996 if (retval)
997 goto out_free_dentry;
998
999 /* Flush all traces of the currently running executable */
1000 retval = begin_new_exec(bprm);
1001 if (retval)
1002 goto out_free_dentry;
1003
1004 /* Do this immediately, since STACK_TOP as used in setup_arg_pages
1005 may depend on the personality. */
1006 SET_PERSONALITY2(*elf_ex, &arch_state);
1007 if (elf_read_implies_exec(*elf_ex, executable_stack))
1008 current->personality |= READ_IMPLIES_EXEC;
1009
1010 if (!(current->personality & ADDR_NO_RANDOMIZE) && randomize_va_space)
1011 current->flags |= PF_RANDOMIZE;
1012
1013 setup_new_exec(bprm);
1014
1015 /* Do this so that we can load the interpreter, if need be. We will
1016 change some of these later */
1017 retval = setup_arg_pages(bprm, randomize_stack_top(STACK_TOP),
1018 executable_stack);
1019 if (retval < 0)
1020 goto out_free_dentry;
1021
1022 elf_bss = 0;
1023 elf_brk = 0;
1024
1025 start_code = ~0UL;
1026 end_code = 0;
1027 start_data = 0;
1028 end_data = 0;
1029
1030 /* Now we do a little grungy work by mmapping the ELF image into
1031 the correct location in memory. */
1032 for(i = 0, elf_ppnt = elf_phdata;
1033 i < elf_ex->e_phnum; i++, elf_ppnt++) {
1034 int elf_prot, elf_flags;
1035 unsigned long k, vaddr;
1036 unsigned long total_size = 0;
1037 unsigned long alignment;
1038
1039 if (elf_ppnt->p_type != PT_LOAD)
1040 continue;
1041
1042 if (unlikely (elf_brk > elf_bss)) {
1043 unsigned long nbyte;
1044
1045 /* There was a PT_LOAD segment with p_memsz > p_filesz
1046 before this one. Map anonymous pages, if needed,
1047 and clear the area. */
1048 retval = set_brk(elf_bss + load_bias,
1049 elf_brk + load_bias,
1050 bss_prot);
1051 if (retval)
1052 goto out_free_dentry;
1053 nbyte = ELF_PAGEOFFSET(elf_bss);
1054 if (nbyte) {
1055 nbyte = ELF_MIN_ALIGN - nbyte;
1056 if (nbyte > elf_brk - elf_bss)
1057 nbyte = elf_brk - elf_bss;
1058 if (clear_user((void __user *)elf_bss +
1059 load_bias, nbyte)) {
1060 /*
1061 * This bss-zeroing can fail if the ELF
1062 * file specifies odd protections. So
1063 * we don't check the return value
1064 */
1065 }
1066 }
1067 }
1068
1069 elf_prot = make_prot(elf_ppnt->p_flags, &arch_state,
1070 !!interpreter, false);
1071
1072 elf_flags = MAP_PRIVATE;
1073
1074 vaddr = elf_ppnt->p_vaddr;
1075 /*
1076 * The first time through the loop, load_addr_set is false:
1077 * layout will be calculated. Once set, use MAP_FIXED since
1078 * we know we've already safely mapped the entire region with
1079 * MAP_FIXED_NOREPLACE in the once-per-binary logic following.
1080 */
1081 if (load_addr_set) {
1082 elf_flags |= MAP_FIXED;
1083 } else if (elf_ex->e_type == ET_EXEC) {
1084 /*
1085 * This logic is run once for the first LOAD Program
1086 * Header for ET_EXEC binaries. No special handling
1087 * is needed.
1088 */
1089 elf_flags |= MAP_FIXED_NOREPLACE;
1090 } else if (elf_ex->e_type == ET_DYN) {
1091 /*
1092 * This logic is run once for the first LOAD Program
1093 * Header for ET_DYN binaries to calculate the
1094 * randomization (load_bias) for all the LOAD
1095 * Program Headers.
1096 *
1097 * There are effectively two types of ET_DYN
1098 * binaries: programs (i.e. PIE: ET_DYN with INTERP)
1099 * and loaders (ET_DYN without INTERP, since they
1100 * _are_ the ELF interpreter). The loaders must
1101 * be loaded away from programs since the program
1102 * may otherwise collide with the loader (especially
1103 * for ET_EXEC which does not have a randomized
1104 * position). For example to handle invocations of
1105 * "./ld.so someprog" to test out a new version of
1106 * the loader, the subsequent program that the
1107 * loader loads must avoid the loader itself, so
1108 * they cannot share the same load range. Sufficient
1109 * room for the brk must be allocated with the
1110 * loader as well, since brk must be available with
1111 * the loader.
1112 *
1113 * Therefore, programs are loaded offset from
1114 * ELF_ET_DYN_BASE and loaders are loaded into the
1115 * independently randomized mmap region (0 load_bias
1116 * without MAP_FIXED nor MAP_FIXED_NOREPLACE).
1117 */
1118 if (interpreter) {
1119 load_bias = ELF_ET_DYN_BASE;
1120 if (current->flags & PF_RANDOMIZE)
1121 load_bias += arch_mmap_rnd();
1122 alignment = maximum_alignment(elf_phdata, elf_ex->e_phnum);
1123 if (alignment)
1124 load_bias &= ~(alignment - 1);
1125 elf_flags |= MAP_FIXED_NOREPLACE;
1126 } else
1127 load_bias = 0;
1128
1129 /*
1130 * Since load_bias is used for all subsequent loading
1131 * calculations, we must lower it by the first vaddr
1132 * so that the remaining calculations based on the
1133 * ELF vaddrs will be correctly offset. The result
1134 * is then page aligned.
1135 */
1136 load_bias = ELF_PAGESTART(load_bias - vaddr);
1137 }
1138
1139 /*
1140 * Calculate the entire size of the ELF mapping (total_size).
1141 * (Note that load_addr_set is set to true later once the
1142 * initial mapping is performed.)
1143 */
1144 if (!load_addr_set) {
1145 total_size = total_mapping_size(elf_phdata,
1146 elf_ex->e_phnum);
1147 if (!total_size) {
1148 retval = -EINVAL;
1149 goto out_free_dentry;
1150 }
1151 }
1152
1153 error = elf_map(bprm->file, load_bias + vaddr, elf_ppnt,
1154 elf_prot, elf_flags, total_size);
1155 if (BAD_ADDR(error)) {
1156 retval = IS_ERR((void *)error) ?
1157 PTR_ERR((void*)error) : -EINVAL;
1158 goto out_free_dentry;
1159 }
1160
1161 if (!load_addr_set) {
1162 load_addr_set = 1;
1163 load_addr = (elf_ppnt->p_vaddr - elf_ppnt->p_offset);
1164 if (elf_ex->e_type == ET_DYN) {
1165 load_bias += error -
1166 ELF_PAGESTART(load_bias + vaddr);
1167 load_addr += load_bias;
1168 reloc_func_desc = load_bias;
1169 }
1170 }
1171
1172 if (elf_ppnt->p_offset <= elf_ex->e_phoff &&
1173 elf_ex->e_phoff < elf_ppnt->p_offset + elf_ppnt->p_filesz) {
1174 phdr_addr = elf_ex->e_phoff - elf_ppnt->p_offset +
1175 elf_ppnt->p_vaddr;
1176 }
1177
1178 k = elf_ppnt->p_vaddr;
1179 if ((elf_ppnt->p_flags & PF_X) && k < start_code)
1180 start_code = k;
1181 if (start_data < k)
1182 start_data = k;
1183
1184 /*
1185 * Check to see if the section's size will overflow the
1186 * allowed task size. Note that p_filesz must always be
1187 * <= p_memsz so it is only necessary to check p_memsz.
1188 */
1189 if (BAD_ADDR(k) || elf_ppnt->p_filesz > elf_ppnt->p_memsz ||
1190 elf_ppnt->p_memsz > TASK_SIZE ||
1191 TASK_SIZE - elf_ppnt->p_memsz < k) {
1192 /* set_brk can never work. Avoid overflows. */
1193 retval = -EINVAL;
1194 goto out_free_dentry;
1195 }
1196
1197 k = elf_ppnt->p_vaddr + elf_ppnt->p_filesz;
1198
1199 if (k > elf_bss)
1200 elf_bss = k;
1201 if ((elf_ppnt->p_flags & PF_X) && end_code < k)
1202 end_code = k;
1203 if (end_data < k)
1204 end_data = k;
1205 k = elf_ppnt->p_vaddr + elf_ppnt->p_memsz;
1206 if (k > elf_brk) {
1207 bss_prot = elf_prot;
1208 elf_brk = k;
1209 }
1210 }
1211
1212 e_entry = elf_ex->e_entry + load_bias;
1213 phdr_addr += load_bias;
1214 elf_bss += load_bias;
1215 elf_brk += load_bias;
1216 start_code += load_bias;
1217 end_code += load_bias;
1218 start_data += load_bias;
1219 end_data += load_bias;
1220
1221 /* Calling set_brk effectively mmaps the pages that we need
1222 * for the bss and break sections. We must do this before
1223 * mapping in the interpreter, to make sure it doesn't wind
1224 * up getting placed where the bss needs to go.
1225 */
1226 retval = set_brk(elf_bss, elf_brk, bss_prot);
1227 if (retval)
1228 goto out_free_dentry;
1229 if (likely(elf_bss != elf_brk) && unlikely(padzero(elf_bss))) {
1230 retval = -EFAULT; /* Nobody gets to see this, but.. */
1231 goto out_free_dentry;
1232 }
1233
1234 if (interpreter) {
1235 elf_entry = load_elf_interp(interp_elf_ex,
1236 interpreter,
1237 load_bias, interp_elf_phdata,
1238 &arch_state);
1239 if (!IS_ERR((void *)elf_entry)) {
1240 /*
1241 * load_elf_interp() returns relocation
1242 * adjustment
1243 */
1244 interp_load_addr = elf_entry;
1245 elf_entry += interp_elf_ex->e_entry;
1246 }
1247 if (BAD_ADDR(elf_entry)) {
1248 retval = IS_ERR((void *)elf_entry) ?
1249 (int)elf_entry : -EINVAL;
1250 goto out_free_dentry;
1251 }
1252 reloc_func_desc = interp_load_addr;
1253
1254 allow_write_access(interpreter);
1255 fput(interpreter);
1256
1257 kfree(interp_elf_ex);
1258 kfree(interp_elf_phdata);
1259 } else {
1260 elf_entry = e_entry;
1261 if (BAD_ADDR(elf_entry)) {
1262 retval = -EINVAL;
1263 goto out_free_dentry;
1264 }
1265 }
1266
1267 kfree(elf_phdata);
1268
1269 set_binfmt(&elf_format);
1270

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]