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

[Sascha Hauer <s.hauer@pengutronix.de>]

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.

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