Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756365AbbGTQx2 (ORCPT ); Mon, 20 Jul 2015 12:53:28 -0400 Received: from mail-ig0-f193.google.com ([209.85.213.193]:33223 "EHLO mail-ig0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753789AbbGTQx0 (ORCPT ); Mon, 20 Jul 2015 12:53:26 -0400 MIME-Version: 1.0 In-Reply-To: References: Date: Tue, 21 Jul 2015 01:53:25 +0900 X-Google-Sender-Auth: LdoA2-VW3qA4foxJfFVWTpYvvao Message-ID: Subject: Re: [PATCH v7 2/4] x86/stackvalidate: Compile-time stack validation From: Namhyung Kim To: Josh Poimboeuf Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Michal Marek , Peter Zijlstra , Andy Lutomirski , Borislav Petkov , Linus Torvalds , Andi Kleen , Pedro Alves , x86@kernel.org, live-patching@vger.kernel.org, LKML Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3549 Lines: 144 Hi, Just nitpicks below.. On Wed, Jul 15, 2015 at 2:14 AM, Josh Poimboeuf wrote: > +struct elf *elf_open(const char *name) > +{ > + struct elf *elf; > + > + elf_version(EV_CURRENT); > + > + elf = malloc(sizeof(*elf)); > + if (!elf) { > + perror("malloc"); > + return NULL; > + } > + memset(elf, 0, sizeof(*elf)); > + > + INIT_LIST_HEAD(&elf->sections); > + > + elf->name = strdup(name); > + if (!elf->name) { > + perror("strdup"); > + goto err; > + } > + > + elf->fd = open(name, O_RDONLY); > + if (elf->fd == -1) { > + perror("open"); > + goto err; > + } > + > + elf->elf = elf_begin(elf->fd, ELF_C_READ_MMAP, NULL); > + if (!elf->elf) { > + perror("elf_begin"); > + goto err; > + } > + > + if (!gelf_getehdr(elf->elf, &elf->ehdr)) { > + perror("gelf_getehdr"); > + goto err; > + } > + > + if (read_sections(elf)) > + goto err; > + > + if (read_symbols(elf)) > + goto err; > + > + if (read_relas(elf)) > + goto err; > + > + return elf; > + > +err: > + elf_close(elf); > + return NULL; > +} > + > +void elf_close(struct elf *elf) > +{ > + struct section *sec, *tmpsec; > + struct symbol *sym, *tmpsym; > + > + list_for_each_entry_safe(sec, tmpsec, &elf->sections, list) { > + list_for_each_entry_safe(sym, tmpsym, &sec->symbols, list) { > + list_del(&sym->list); > + free(sym); > + } It seems that it needs to free entries in sec->relas too here. > + list_del(&sec->list); > + free(sec); > + } > + if (elf->name) > + free(elf->name); > + if (elf->fd > 0) > + close(elf->fd); > + if (elf->elf) > + elf_end(elf->elf); > + free(elf); > +} [SNIP] > +int main(int argc, char *argv[]) > +{ > + struct elf *elf; > + int ret = 0, warnings = 0; > + > + argp_parse(&argp, argc, argv, 0, 0, &args); > + > + elf = elf_open(args.args[0]); > + if (!elf) { > + fprintf(stderr, "error reading elf file %s\n", args.args[0]); > + return 1; > + } > + > + ret = decode_sections(elf); > + if (ret < 0) > + goto out; > + warnings += ret; > + > + ret = validate_functions(elf); > + if (ret < 0) > + goto out; > + warnings += ret; > + > + ret = validate_uncallable_instructions(elf); > + if (ret < 0) > + goto out; > + warnings += ret; > + > +out: elf_close(elf); ?? Thanks, Namhyung > + /* ignore warnings for now until we get all the code cleaned up */ > + if (ret || warnings) > + return 0; > + return 0; > +} > -- > 2.1.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/