IPVS: replace sprintf to snprintf to avoid stack buffer overflow
in [Kernel]
|
Prev: Failed to initialize MSI interrupts && ioremap reserve_memtype failed -22
Next: Add a tunable that decides when memory should be compacted and when it should be reclaimed
From: Patrick McHardy on 7 Apr 2010 12:20 Simon Horman wrote: > On Tue, Apr 06, 2010 at 10:50:20AM +0800, wzt.wzt(a)gmail.com wrote: >> IPVS not check the length of pp->name, use sprintf will cause stack buffer overflow. >> struct ip_vs_protocol{} declare name as char *, if register a protocol as: >> struct ip_vs_protocol ip_vs_test = { >> .name = "aaaaaaaa....128...aaa", >> .debug_packet = ip_vs_tcpudp_debug_packet, >> }; >> >> when called ip_vs_tcpudp_debug_packet(), sprintf(buf, "%s TRUNCATED", pp->name); >> will cause stack buffer overflow. >> >> Signed-off-by: Zhitong Wang <zhitong.wangzt(a)alibaba-inc.com> > > I think that the simple answer is, don't do that. Indeed. > But your patch seems entirely reasonable to me. > > Acked-by: Simon Horman <horms(a)verge.net.au> > > Patrick, please consider merging this. I think this fix is a bit silly, we can simply print the name in the pr_debug() statement and avoid both the potential overflow and truncation. How does this look?
From: Patrick McHardy on 8 Apr 2010 07:40
Simon Horman wrote: > On Wed, Apr 07, 2010 at 06:09:54PM +0200, Patrick McHardy wrote: >> Simon Horman wrote: >>> On Tue, Apr 06, 2010 at 10:50:20AM +0800, wzt.wzt(a)gmail.com wrote: >>>> IPVS not check the length of pp->name, use sprintf will cause stack buffer overflow. >>>> struct ip_vs_protocol{} declare name as char *, if register a protocol as: >>>> struct ip_vs_protocol ip_vs_test = { >>>> .name = "aaaaaaaa....128...aaa", >>>> .debug_packet = ip_vs_tcpudp_debug_packet, >>>> }; >>>> >>>> when called ip_vs_tcpudp_debug_packet(), sprintf(buf, "%s TRUNCATED", pp->name); >>>> will cause stack buffer overflow. >>>> >>>> Signed-off-by: Zhitong Wang <zhitong.wangzt(a)alibaba-inc.com> >>> I think that the simple answer is, don't do that. >> Indeed. >> >>> But your patch seems entirely reasonable to me. >>> >>> Acked-by: Simon Horman <horms(a)verge.net.au> >>> >>> Patrick, please consider merging this. >> I think this fix is a bit silly, we can simply print the name in >> the pr_debug() statement and avoid both the potential overflow >> and truncation. >> >> How does this look? > > Looks good to me: > > Acked-by: Simon Horman <horms(a)verge.net.au> Thanks, I've applied the patch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |