[SLOF] [PATCH v2 09/11] libnet: Add support for DHCPv4 options 209 and 210

[Thomas Huth]

On 25.05.2018 08:47, Alexey Kardashevskiy wrote:
> On 19/5/18 1:45 am, Thomas Huth wrote:
>> There are two dedicated DHCP options for loading PXELINUX config files,
>> option 209 (config file name) and 210 (path prefix). We should support
>> them, too, in case some users want to configure their boot flow this way.
>> See RFC 5071 and the following URL for more details:
>> https://www.syslinux.org/wiki/index.php?title=PXELINUX#DHCP_options
>>
>> Unlike most other strings in libnet, I've chosen to not use fixed-size
>> arrays for these two strings, but to allocate the memory via malloc here.
>> We always have to make sure not to overflow the stack in Paflof, so
>> adding 2 * 256 byte arrays to struct filename_ip sounded just too
>> dangerous to me.
>>
>> Signed-off-by: Thomas Huth <thuth at redhat.com>
>> ---
>>  lib/libnet/dhcp.c     | 33 +++++++++++++++++++++++++++++++++
>>  lib/libnet/netload.c  |  7 ++++++-
>>  lib/libnet/ping.c     | 14 +++++++++-----
>>  lib/libnet/pxelinux.c | 24 +++++++++++++++++++-----
>>  lib/libnet/tftp.h     |  2 ++
>>  5 files changed, 69 insertions(+), 11 deletions(-)
>>
>> diff --git a/lib/libnet/dhcp.c b/lib/libnet/dhcp.c
>> index d3e5170..85cd7c0 100644
>> --- a/lib/libnet/dhcp.c
>> +++ b/lib/libnet/dhcp.c
>> @@ -79,6 +79,8 @@
>>  #define DHCP_TFTP_SERVER      66
>>  #define DHCP_BOOTFILE         67
>>  #define DHCP_CLIENT_ARCH      93
>> +#define DHCP_PXELINUX_CFGFILE 209   /* See RFC 5071 */
>> +#define DHCP_PXELINUX_PREFIX  210
>>  #define DHCP_ENDOPT         0xFF
>>  #define DHCP_PADOPT         0x00
>>  
>> @@ -167,6 +169,8 @@ static uint32_t dhcp_siaddr_ip     = 0;
>>  static char   dhcp_filename[256];
>>  static char   dhcp_tftp_name[256];
>>  static uint32_t dhcp_xid;
>> +static char *pxelinux_cfgfile;
>> +static char *pxelinux_prefix;
> 
> Why are not these in dhcp_options_t?

That whole dhcp code is a little bit strange: dhcp_options_t is only
"valid" during handle_dhcp(), so it finally copies the options also into
the static char dhcp_filename[256] and the static char
dhcp_tftp_name[256] - and dhcpv4() finally copies the stuff over into
filename_ip_t. So the "static" variables are urgently needed for the
currently logic here, while an entry in dhcp_options_t is just optional.
Thus let's keep it simple and only use the static variables here.

 Thomas


PS:
If somebody gets bored, I think a major refactoring of the dhcp code
could make this dhcp.c file way more readable. E.g. I also have not
understood yet why dhcp_encode_options() is checking flag[DHCP_BOOTFILE]
and flag[DHCP_TFTP_SERVER] for example...