lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 17 Jul 2014 14:52:46 +0100 From: Markos Chandras <Markos.Chandras@...tec.com> To: Andrey Utkin <andrey.krieger.utkin@...il.com>, <linux-kernel@...r.kernel.org>, <kernel-janitors@...r.kernel.org>, <linux-mips@...ux-mips.org> CC: <dborkman@...hat.com>, <ralf@...ux-mips.org> Subject: Re: [PATCH 2/3] arch/mips/net/bpf_jit.c: fix failure check On 07/17/2014 02:27 PM, Andrey Utkin wrote: > static int pkt_type_offset(void) returned -1 in case of failure, and > actual (positive) offset value in case of success. In the only instance > of its usage, the result was saved to local "unsigned int off" variable, > which is used in a lot of places in the same (large) function, so > changing its type could cause many warnings. > There was no signed int variable which could be just used for this case. > There are two possibilities to resolve that: to declare temporary signed > int variable to get the return value from pkt_type_offset(), or to > separate return status from returned offset value. The latter approach > was chosen, however, I am not sure which would be practically optimal. > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=80371 > Reported-by: David Binderman <dcb314@...mail.com> > Signed-off-by: Andrey Utkin <andrey.krieger.utkin@...il.com> > --- > arch/mips/net/bpf_jit.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/arch/mips/net/bpf_jit.c b/arch/mips/net/bpf_jit.c > index b87390a..47f65d5 100644 > --- a/arch/mips/net/bpf_jit.c > +++ b/arch/mips/net/bpf_jit.c > @@ -770,7 +770,7 @@ static u64 jit_get_skb_w(struct sk_buff *skb, unsigned offset) > #else > #define PKT_TYPE_MAX 7 > #endif > -static int pkt_type_offset(void) > +static int pkt_type_offset(unsigned int *off_arg) > { > struct sk_buff skb_probe = { > .pkt_type = ~0, > @@ -779,8 +779,10 @@ static int pkt_type_offset(void) > unsigned int off; > > for (off = 0; off < sizeof(struct sk_buff); off++) { > - if (ct[off] == PKT_TYPE_MAX) > - return off; > + if (ct[off] == PKT_TYPE_MAX) { > + *off_arg = off; > + return 0; > + } > } > pr_err_once("Please fix pkt_type_offset(), as pkt_type couldn't be found\n"); > return -1; > @@ -1332,9 +1334,7 @@ jmp_cmp: > case BPF_ANC | SKF_AD_PKTTYPE: > ctx->flags |= SEEN_SKB; > > - off = pkt_type_offset(); > - > - if (off < 0) > + if (pkt_type_offset(&off)) > return -1; > emit_load_byte(r_tmp, r_skb, off, ctx); > /* Keep only the last 3 bits */ > Hi, Thanks for the patch. I would personally prefer to use a new signed int variable, but I am fine either way. -- markos -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists