Re: [PATCH 3/8] net: dhcp: add support of tftp server ip or Etherboot file (option 150)

[Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>]

On 12:49 Mon 02 Apr     , Sascha Hauer wrote:
> On Fri, Mar 30, 2012 at 12:14:32PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 11:21 Fri 30 Mar     , Sascha Hauer wrote:
> > > >  					setenv("bootfile", str);
> > > >  			}
> > > >  			break;
> > > > +		case 150:	/* TFTP server ip or Etherboot*/
> > > > +			if (oplen == sizeof(IPaddr_t)) {
> > > > +				is_tftpserverip = true;
> > > > +				ip = net_read_ip(popt + 2);
> > > > +				net_set_serverip(ip);
> > > > +				setenv_ip("tftp_server_ip", ip);
> > > > +			} else {
> > > > +				memcpy(str, popt + 2, oplen);
> > > > +				str[oplen] = 0;
> > > > +				setenv("etherboot_file", str);
> > > > +			}
> > > 
> > > This seems fragile. What if the etherboot filename has a length of 4?
> > agree but this is really unlikely and I did want to add an option in the
> > client to ask a file name
> > 
> > And we can not detect it via the DHCP RFC
> 
> For tftp server ip it's not one server ip but a list of server ips, so
> the check for sizeof(IPaddr_t) is not even correct, you must check for
> multiple of sizeof(IPaddr_t) which makes the above code even more
> fragile. No way to accept this.
yeah because option 150 is use by etherboot when they do not respect the RFC
and nearly no dhcp server send more than one server ip via options 150

Best Regards,
J.

> 
> Sascha
> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox