Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 16 Jun 2020 12:51:06 -0700
From:   Matt Helsley <mhelsley@vmware.com>
To:     Peter Zijlstra <peterz@infradead.org>
CC:     Josh Poimboeuf <jpoimboe@redhat.com>,
        <linux-kernel@vger.kernel.org>, <x86@kernel.org>,
        <dvyukov@google.com>, <elver@google.com>, <andreyknvl@google.com>,
        <mark.rutland@arm.com>, <rostedt@goodmis.org>,
        <jthierry@redhat.com>, <mbenes@suse.cz>
Subject: Re: [RFC][PATCH 2/3] objtool: Provide elf_write_{insn,reloc}()
Message-ID: <20200616195106.GE25598@rlwimi.vmware.com>
Mail-Followup-To: Matt Helsley <mhelsley@vmware.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Josh Poimboeuf <jpoimboe@redhat.com>, linux-kernel@vger.kernel.org,
        x86@kernel.org, dvyukov@google.com, elver@google.com,
        andreyknvl@google.com, mark.rutland@arm.com, rostedt@goodmis.org,
        jthierry@redhat.com, mbenes@suse.cz
References: <20200612143034.933422660@infradead.org>
 <20200612143554.012293047@infradead.org>
 <20200616091253.GN2531@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20200616091253.GN2531@hirez.programming.kicks-ass.net>
Received-SPF: None (EX13-EDG-OU-002.vmware.com: mhelsley@vmware.com does not
 designate permitted sender hosts)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Jun 16, 2020 at 11:12:53AM +0200, Peter Zijlstra wrote:
> On Fri, Jun 12, 2020 at 04:30:36PM +0200, Peter Zijlstra wrote:
> > +int elf_write_insn(struct elf *elf, struct section *sec,
> > +		   unsigned long offset, unsigned int len,
> > +		   const char *insn)
> > +{
> > +	Elf_Data *data = sec->data;
> > +
> > +	if (data->d_type != ELF_T_BYTE || data->d_off) {
> > +		WARN("write to unexpected data for section: %s", sec->name);
> > +		return -1;
> > +	}
> > +
> > +	memcpy(data->d_buf + offset, insn, len);
> > +	elf_flagdata(data, ELF_C_SET, ELF_F_DIRTY);
> > +
> > +	sec->changed = true;
> > +	elf->changed = true;
> > +
> > +	return 0;
> > +}
> > +
> > +int elf_write_reloc(struct elf *elf, struct reloc *reloc)
> > +{
> > +	struct section *sec = reloc->sec;
> > +
> > +	if (sec->sh.sh_type == SHT_REL) {
> > +		reloc->rel.r_info = GELF_R_INFO(reloc->sym->idx, reloc->type);
> > +		reloc->rel.r_offset = reloc->offset;
> > +
> > +		if (!gelf_update_rel(sec->data, reloc->idx, &reloc->rel)) {
> > +			WARN_ELF("gelf_update_rel");
> > +			return -1;
> > +		}
> > +	} else {
> > +		reloc->rela.r_info = GELF_R_INFO(reloc->sym->idx, reloc->type);
> > +		reloc->rela.r_addend = reloc->addend;
> > +		reloc->rela.r_offset = reloc->offset;
> > +
> > +		if (!gelf_update_rela(sec->data, reloc->idx, &reloc->rela)) {
> > +			WARN_ELF("gelf_update_rela");
> > +			return -1;
> > +		}
> > +	}
> > +
> > +	sec->changed = true;
> > +	elf->changed = true;
> > +
> > +	return 0;
> > +}
> 
> Doing the change Matt asked for #1, I realized that sec->changed is only
> required if we need to rewrite the section header, neither of these two
> changes requires that, they already mark the elf data dirty so
> elf_update() DTRT.

This is really useful information.

As long as you're adding the elf->changed flag it might make sense to add this
as a comment in the struct section definition or even rename sec->changed
to reflect this (e.g. sec->shdr_changed).

Cheers,
     -Matt Helsley